test: keep cif structs alive during gc #380
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Up until now, test/callback.js assumed that the `cb.cif` object would not be garbage collected and was available to `Callback::Invoke`. That has never been a valid assumption, but as of nodejs/node#12141 Buffers created with `new Buffer(n)` each have their own `ArrayBuffer` which gets garbage-collected much more easily, which in turn would crash the test suite here. To (lazy-)fix this, assign `cb._cif` to some global variable that is guaranteed to stay alive.
4 tasks
|
Ping @TooTallNate ? It would be cool to see this merged, or addressed in some other way, otherwise I fear that this module might need to be removed from CITGM coverage… |
|
I'm sorry for not getting to this sooner. I'm not a fan of this (lazy-)fix, so this fell off of my radar. Really this should be fixed in |
addaleax
added a commit
to node-ffi-napi/node-ffi
that referenced
this pull request
Nov 27, 2017
Up until now, test/callback.js assumed that the `cb.cif` object would not be garbage collected and was available to `Callback::Invoke`. That has never been a valid assumption, but as of nodejs/node#12141 Buffers created with `new Buffer(n)` each have their own `ArrayBuffer` which gets garbage-collected much more easily, which in turn would crash the test suite here. To (lazy-)fix this, assign `cb._cif` to some global variable that is guaranteed to stay alive. PR-URL: node-ffi#380
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Up until now, test/callback.js assumed that the
cb.cifobjectwould not be garbage collected and was available to
Callback::Invoke.That has never been a valid assumption, but as of
nodejs/node#12141 Buffers created with
new Buffer(n)each have their ownArrayBufferwhich getsgarbage-collected much more easily, which in turn would
crash the test suite here.
To (lazy-)fix this, assign
cb._cifto some global variable that isguaranteed to stay alive.