dcSpark / cardano-multiplatform-lib

Rust implementation of Cardano
MIT License
99 stars 36 forks source link

RuntimeError: memory access out of bounds #330

Closed solidsnakedev closed 6 months ago

solidsnakedev commented 6 months ago

I'm running into some issues when running some tests

RuntimeError: memory access out of bounds
@lucid-evolution/lucid:test:  ❯ null.<anonymous> wasm:/wasm/00a747c2:1:670612
@lucid-evolution/lucid:test:  ❯ null.<anonymous> wasm:/wasm/00a747c2:1:2248216
@lucid-evolution/lucid:test:  ❯ null.<anonymous> wasm:/wasm/00a747c2:1:2240349
@lucid-evolution/lucid:test:  ❯ null.<anonymous> wasm:/wasm/00a747c2:1:1542408
@lucid-evolution/lucid:test:  ❯ null.<anonymous> wasm:/wasm/00a747c2:1:2201648
@lucid-evolution/lucid:test:  ❯ null.<anonymous> wasm:/wasm/00a747c2:1:2231342
@lucid-evolution/lucid:test:  ❯ ../../node_modules/.pnpm/@dcspark+cardano-multiplatform-lib-nodejs@5.3.0/node_modules/@dcspark/cardano-multiplatform-lib-nodejs/cardano_multiplatform_lib.js:11219:70

I think the issues has to do with rust releasing memory,

This CML error points to line 11219 which is const DatumOptionFinalization = new FinalizationRegistry(ptr => wasm.__wbg_datumoption_free(ptr));

I've also had to manually free CML.TransactionBuilder when is not longer needed, along with CML.TransactionBuilderConfig

rooooooooob commented 6 months ago

Do you have any tight loops that run many, many times that create anything in CML? We've enabled weakrefs in wasm/js which are supposed to be freed automatically when they go out of scope but I think JS gets behind on this sometimes so anywhere that is creating lots of the same struct you might want to explicitly call free().

When I tested this I had the tx builder create millions of transactions in a loop and it'd get out of memory after I think 500k if I had it in a regular loop, but if I put some pauses in it it'd still eventually run out of memory but usually not until after many millions. The more pause I gave it the less likely it was to do that but it'd still tend to happen when I was doing it with like 50k iterations before pausing.

And are you sure nothing in JS is maintaining references to any of these wasm structs? That would stop it from being automatically GC'd too. I'm not sure if it's smart enough to detect things that reference each other and nothing else or if it just counts refs but it could be the latter.

solidsnakedev commented 6 months ago

I have implemented a retry loop in case the transaction fails, but it is currently configured for only four attempts, with exponential delays of 20 seconds.

Now, I have made the following updates: Updated CML to version 5.3. Upgraded vitest to version 1.6.0. Updated Node.js to version 21.6.2.

and I get this error...

@lucid-evolution/lucid:test: Error: recursive use of an object detected which would lead to unsafe aliasing in rust
@lucid-evolution/lucid:test:  ❯ module.exports.__wbindgen_throw ../../node_modules/.pnpm/@dcspark+cardano-multiplatform-lib-nodejs@5.3.0/node_modules/@dcspark/cardano-multiplatform-lib-nodejs/cardano_multiplatform_lib.js:37518:11
@lucid-evolution/lucid:test:  ❯ null.<anonymous> wasm:/wasm/00a747c2:1:2501206
@lucid-evolution/lucid:test:  ❯ null.<anonymous> wasm:/wasm/00a747c2:1:2501220
@lucid-evolution/lucid:test:  ❯ null.<anonymous> wasm:/wasm/00a747c2:1:2212507
@lucid-evolution/lucid:test:  ❯ null.<anonymous> wasm:/wasm/00a747c2:1:2034126
@lucid-evolution/lucid:test:  ❯ null.<anonymous> wasm:/wasm/00a747c2:1:2231337
@lucid-evolution/lucid:test:  ❯ ../../node_modules/.pnpm/@dcspark+cardano-multiplatform-lib-nodejs@5.3.0/node_modules/@dcspark/cardano-multiplatform-lib-nodejs/cardano_multiplatform_lib.js:11219:70
@lucid-evolution/lucid:test: 
@lucid-evolution/lucid:test: This error originated in "test/hello.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
@lucid-evolution/lucid:test: The latest test that might've caused the error is "CollectFunds". It might mean one of the following:
@lucid-evolution/lucid:test: - The error was thrown, while Vitest was running this test.
@lucid-evolution/lucid:test: - If the error occurred after the test had been completed, this was the last documented test before it was thrown.

I think this error relates to this? https://github.com/rustwasm/wasm-bindgen/issues/1578 Error: recursive use of an object detected which would lead to unsafe aliasing in rust

solidsnakedev commented 6 months ago

Is it possible to return more information when an error occurs, like using something such as https://github.com/rustwasm/console_error_panic_hook , @rooooooooob ?

solidsnakedev commented 6 months ago

I'm also wondering if CML is using the latest https://github.com/rustwasm/wasm-bindgen/releases/tag/0.2.92 , @rooooooooob ?

solidsnakedev commented 6 months ago

I think the problem has to do with DatumOption, for some reason when I free() DatumOption it returns this error Error: null pointer passed to rust'

However when I remove free() , this error keeps coming back image

solidsnakedev commented 6 months ago

@rooooooooob @SebastienGllmt I managed to fix the error by manipulating the file. node_modules/.pnpm/@dcspark+cardano-multiplatform-lib-nodejs@5.3.0/node_modules/@dcspark/cardano-multiplatform-lib-nodejs/cardano_multiplatform_lib.js

I commented out these 2 lines DatumOptionFinalization.register(obj, obj.ptr, obj); DatumOptionFinalization.unregister(this);

My question is, can we disable them on demand?

image

SebastienGllmt commented 6 months ago

That code is generated by wasm-pack tooling. Unless you create a script to modify the CML outputs, you would have to go make a PR to the underlying wasm pack library

solidsnakedev commented 6 months ago

I've noticed that when I replaced TransactionOutput.new(...) with TransactionOutputBuilder.new(), the error no longer occurred. I'll continue testing, and perhaps TransactionOutput.new should be deprecated or completely replaced by TransactionOutputBuilder.

solidsnakedev commented 6 months ago

I think I can close this, I've been testing the TransactionOutputBuilder.new() without any issues at all