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.

Sign up for GitHub

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

Jump to bottom

n-api - failure in node-addon-api testing after change in node core #19673

Closed
mhdawson opened this issue Mar 29, 2018 · 6 comments
Closed

n-api - failure in node-addon-api testing after change in node core #19673

mhdawson opened this issue Mar 29, 2018 · 6 comments
Labels
node-api Issues and PRs related to the Node-API.

Comments

@mhdawson
Copy link
Member

  • Version: master
  • Platform: all
  • Subsystem: n-api

We are seeing the following failure:

https://ci.nodejs.org/job/node-test-node-addon-api/MACHINE=fedora-last-latest-x64/237/console

All tests passed!
/home/iojs/build/workspace/node-test-node-addon-api/MACHINE/fedora-last-latest-x64/node-v10.0.0-nightly20180328bc690e9ef5-linux-x64/bin/node[26239]: ../src/node_api.cc:466:static void {anonymous}::v8impl::Reference::FinalizeCallback(const v8::WeakCallbackInfo<{anonymous}::v8impl::Reference>&): Assertion `(((reference->_env))->open_handle_scopes) == (open_handle_scopes)' failed.
 1: node::Abort() [/home/iojs/build/workspace/node-test-node-addon-api/MACHINE/fedora-last-latest-x64/node-v10.0.0-nightly20180328bc690e9ef5-linux-x64/bin/node]
 2: 0x90170b [/home/iojs/build/workspace/node-test-node-addon-api/MACHINE/fedora-last-latest-x64/node-v10.0.0-nightly20180328bc690e9ef5-linux-x64/bin/node]
 3: 0x90dbea [/home/iojs/build/workspace/node-test-node-addon-api/MACHINE/fedora-last-latest-x64/node-v10.0.0-nightly20180328bc690e9ef5-linux-x64/bin/node]
 4: v8::internal::GlobalHandles::DispatchPendingPhantomCallbacks(bool) [/home/iojs/build/workspace/node-test-node-addon-api/MACHINE/fedora-last-latest-x64/node-v10.0.0-nightly20180328bc690e9ef5-linux-x64/bin/node]
 5: v8::internal::GlobalHandles::PostGarbageCollectionProcessing(v8::internal::GarbageCollector, v8::GCCallbackFlags) [/home/iojs/build/workspace/node-test-node-addon-api/MACHINE/fedora-last-latest-x64/node-v10.0.0-nightly20180328bc690e9ef5-linux-x64/bin/node]
 6: v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) [/home/iojs/build/workspace/node-test-node-addon-api/MACHINE/fedora-last-latest-x64/node-v10.0.0-nightly20180328bc690e9ef5-linux-x64/bin/node]
 7: 0xe72f68 [/home/iojs/build/workspace/node-test-node-addon-api/MACHINE/fedora-last-latest-x64/node-v10.0.0-nightly20180328bc690e9ef5-linux-x64/bin/node]
 8: v8::internal::Heap::CollectAllGarbage(int, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/home/iojs/build/workspace/node-test-node-addon-api/MACHINE/fedora-last-latest-x64/node-v10.0.0-nightly20180328bc690e9ef5-linux-x64/bin/node]
 9: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) [/home/iojs/build/workspace/node-test-node-addon-api/MACHINE/fedora-last-latest-x64/node-v10.0.0-nightly20180328bc690e9ef5-linux-x64/bin/node]
10: 0xb4199f [/home/iojs/build/workspace/node-test-node-addon-api/MACHINE/fedora-last-latest-x64/node-v10.0.0-nightly20180328bc690e9ef5-linux-x64/bin/node]
11: v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) [/home/iojs/build/workspace/node-test-node-addon-api/MACHINE/fedora-last-latest-x64/node-v10.0.0-nightly20180328bc690e9ef5-linux-x64/bin/node]
12: 0x3c65c870427d
Tests aborted with SIGABRT
npm ERR! Test failed.  See above for more details.
Build step 'Conditional steps (multiple)' marked build as failure
Sending e-mails to: michael_dawson@ca.ibm.com gabriel.schulhof@intel.com Arunesh.Chandra@microsoft.com
Notifying upstream projects of job completion
Finished: FAILURE

I think the most likely candidate is: #19537
@gabrielschulhof can you take a look.
@gabrielschulhof
Copy link
Contributor

gabrielschulhof commented Mar 29, 2018 via email

@addaleax addaleax added the node-api Issues and PRs related to the Node-API. label Mar 29, 2018
@gabrielschulhof
Copy link
Contributor

No, my previous modification is actually failing to heed the comment whereby it's inadvisable to access fields of a Reference instance after the finalize_callback has been called, because the callback might actually destroy the Reference.

@gabrielschulhof gabrielschulhof mentioned this issue Apr 1, 2018
Closed
2 tasks
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Apr 2, 2018
n-api: back up env before finalize
225dbeb 
Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: nodejs#19673
@mhdawson
Copy link
Member Author

mhdawson commented Apr 2, 2018

@gabrielschulhof are you working on a fix? Would like to get the node-add-api tests back to green so may take a look if you are not going to have time this week.

@gabrielschulhof
Copy link
Contributor

@mhdawson I'm about to land #19718.

@gabrielschulhof
Copy link
Contributor

@mhdawson I actually have to let it sit for another day, because it's only been two days since I posted the PR.

targos pushed a commit that referenced this issue Apr 3, 2018
@targos
n-api: back up env before finalize
a0a5873 
Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: #19673
PR-URL: #19718
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@mhdawson
Copy link
Member Author

mhdawson commented Apr 4, 2018

@gabrielschulhof thanks, ci is green again :)

gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Apr 12, 2018
n-api: back up env before finalize
e3a2737 
Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: nodejs#19673
PR-URL: nodejs#19718
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Apr 16, 2018
n-api: back up env before finalize
dc4c825 
Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: nodejs#19673
PR-URL: nodejs#19718
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Apr 16, 2018
@MylesBorins
n-api: back up env before finalize
abd9fd6 
Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: #19673
Backport-PR-URL: #19447
PR-URL: #19718
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue May 1, 2018
@MylesBorins
n-api: back up env before finalize
8b3ef46 
Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: #19673
Backport-PR-URL: #19265
PR-URL: #19718
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
node-api Issues and PRs related to the Node-API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

n-api: back up env before finalize gabrielschulhof/node
3 participants
@addaleax @gabrielschulhof @mhdawson