Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dotnet] Root bindings so the delegate in FunctionBinding is not GCed #680

Merged
merged 1 commit into from Dec 6, 2019

Conversation

@AustinWise
Copy link
Contributor

AustinWise commented Dec 6, 2019

Currently Instance does not root the Binding objects. This allows them to be garbage collected. In the case of FunctionBinding, this means its delegate can also be garbage collected. When the WASM runtime calls the reverse P/invoke stub, the CLR notices that the underlying delegate has been garbage collected and exits the process.

@peterhuene peterhuene self-requested a review Dec 6, 2019
@peterhuene peterhuene added the bug label Dec 6, 2019
@peterhuene

This comment has been minimized.

Copy link
Member

peterhuene commented Dec 6, 2019

Nice catch!

I believe I regressed this when I refactored the binding implementation (kept the reference to the delegate from the binding, but broke the reference to the binding that would keep it reachable).

Thanks for fixing this!

@peterhuene peterhuene merged commit 2597468 into bytecodealliance:master Dec 6, 2019
22 checks passed
22 checks passed
Rustfmt
Details
Doc - build the book
Details
Doc - build the API documentation
Details
Fuzz Corpora
Details
Test (stable)
Details
Test (beta)
Details
Test (nightly)
Details
Test (windows)
Details
Test (macos)
Details
Python Wheel (ubuntu-latest)
Details
Python Wheel (macos-latest)
Details
Python Wheel (windows-latest)
Details
Build wasmtime (ubuntu-latest)
Details
Build wasmtime (macos-latest)
Details
Build wasmtime (windows-latest)
Details
Test Wasmtime for .NET bindings (linux-debug)
Details
Test Wasmtime for .NET bindings (linux-release)
Details
Test Wasmtime for .NET bindings (macos-debug)
Details
Test Wasmtime for .NET bindings (macos-release)
Details
Test Wasmtime for .NET bindings (windows-debug)
Details
Test Wasmtime for .NET bindings (windows-release)
Details
Publish
Details
@AustinWise AustinWise deleted the AustinWise:austin/FixGcHole branch Dec 6, 2019
@AustinWise

This comment has been minimized.

Copy link
Contributor Author

AustinWise commented Dec 14, 2019

@peterhuene I think part of the reason it regressed is there are no unit tests for function imports. Made a unit test for this in #718.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.