dart-archive / wasm

Utilities for loading and running WASM modules from Dart code
https://pub.dev/packages/wasm
BSD 3-Clause "New" or "Revised" License
309 stars 25 forks source link

import_error_test crash on exit #47

Open liamappelbe opened 3 years ago

liamappelbe commented 3 years ago

The crash is in the wasm imported function finalizers. With all the finalizers, the isolate shutdown flow is pretty complicated:

Test finishes (Dart code) -> Dart_ShutdownIsolate, finalizes weak handles (Dart VM, C++ code) -> wasm_func_finalizer (package:wasm, C++ code) -> wasm_func_delete (wasmer, Rust code) -> ... -> wasm_func_new_with_env::drop (wasmer, Rust code) -> _wasmFnImportFinalizer (package:wasm, Dart code)

That last step from Rust to Dart is where the crash is happening. I suppose it makes sense that this step would crash if the isolate is shutting down. I don't understand why this is only crashing in this one test though, since every pretty much every test has something like this flow. Need to investigate a bit more.

I've disabled this finalizer for now, since this only leaks a _WasmFnImport, which is small.

liamappelbe commented 3 years ago

Spoke to team. Finalizers should not be calling into Dart, because the GC can finalize objects on a different thread to the Dart code, or at shutdown, so there may not be a Dart context. The Dart API checks this, but FFI doesn't do these checks (it should, but that's a separate issue). So all this means that the finalizer I wrote in Dart may crash randomly.

The fix is to move this last piece of Dart finalizer code into C++, with all the other finalizers. If we can put a Dart handle directly in an FFI object, the _wasmFnImportToFn table can be deleted, and we can just put a persistent handle to the Function directly in the _WasmFnImport, and move the finalizer to native code.