Skip to content
This repository has been archived by the owner on Jul 1, 2023. It is now read-only.

import_error_test crash on exit #47

Open
liamappelbe opened this issue Oct 1, 2021 · 1 comment
Open

import_error_test crash on exit #47

liamappelbe opened this issue Oct 1, 2021 · 1 comment
Labels
bug Something isn't working

Comments

@liamappelbe
Copy link
Contributor

liamappelbe commented Oct 1, 2021

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 liamappelbe added the bug Something isn't working label Oct 1, 2021
@liamappelbe
Copy link
Contributor Author

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Development

No branches or pull requests

1 participant