create a HandleScope in FinalizeCallback #832
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
Contributor Author
| seems like clang-format fails on the whole |
2 tasks
Member
| @blagoev could you add the stack trace to the issue? It would be good to understand the path that leads to the case where there is no HandleScope already on the stack. |
Contributor
| @mhdawson I think this needs to be fixed in core. This fix is certainly not incorrect, but @KevinEady's backtraces in #742 indicate that there is no handle scope when the |
Contributor
| @mhdawson nevermind. This was fixed in core (nodejs/node#34366) in RunAndClearNativeImmediates, which is part of @KevinEady's stack. |
gabrielschulhof approved these changes Nov 19, 2020
Contributor
gabrielschulhof left a comment
gabrielschulhof left a comment There was a problem hiding this comment.
We need this for older versions of Node.js 👍
gabrielschulhof requested review from KevinEady, NickNaso, legendecas and mhdawson Contributor Author
| yes I can confirm the crash was happening when closing the application which might be where |
Member
| I think this is just missing #819 in terms of the lint checks. As far as I can see it passes in my environment. Going to land. |
mhdawson approved these changes Nov 20, 2020
mhdawson pushed a commit that referenced this pull request Nov 20, 2020
Refs: #832 Seems like FinalizeCallback needs to create a HandleScope since it calls ObjectWrap::~ObjectWrap() which might need to create a temporary Object handle here https://github.com/nodejs/node-addon-api/blob/master/napi-inl.h#L3558 at Realm (https://github.com/realm/realm-js) we have crashes with stacktrace at this location. While fixed in core for later versions we still need for older versions of Node.js PR-URL: #832 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Kevin Eady <kevin.c.eady@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: NickNaso <nicoladelgobbo@gmail.com>
Member
| Landed as 1427b3e |
blagoev added a commit to realm/realm-js that referenced this pull request Nov 24, 2020
blagoev added a commit to realm/realm-js that referenced this pull request Nov 25, 2020
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
Refs: nodejs/node-addon-api#832 Seems like FinalizeCallback needs to create a HandleScope since it calls ObjectWrap::~ObjectWrap() which might need to create a temporary Object handle here https://github.com/nodejs/node-addon-api/blob/master/napi-inl.h#L3558 at Realm (https://github.com/realm/realm-js) we have crashes with stacktrace at this location. While fixed in core for later versions we still need for older versions of Node.js PR-URL: nodejs/node-addon-api#832 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Kevin Eady <kevin.c.eady@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: NickNaso <nicoladelgobbo@gmail.com>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022
Refs: nodejs/node-addon-api#832 Seems like FinalizeCallback needs to create a HandleScope since it calls ObjectWrap::~ObjectWrap() which might need to create a temporary Object handle here https://github.com/nodejs/node-addon-api/blob/master/napi-inl.h#L3558 at Realm (https://github.com/realm/realm-js) we have crashes with stacktrace at this location. While fixed in core for later versions we still need for older versions of Node.js PR-URL: nodejs/node-addon-api#832 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Kevin Eady <kevin.c.eady@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: NickNaso <nicoladelgobbo@gmail.com>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022
Refs: nodejs/node-addon-api#832 Seems like FinalizeCallback needs to create a HandleScope since it calls ObjectWrap::~ObjectWrap() which might need to create a temporary Object handle here https://github.com/nodejs/node-addon-api/blob/master/napi-inl.h#L3558 at Realm (https://github.com/realm/realm-js) we have crashes with stacktrace at this location. While fixed in core for later versions we still need for older versions of Node.js PR-URL: nodejs/node-addon-api#832 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Kevin Eady <kevin.c.eady@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: NickNaso <nicoladelgobbo@gmail.com>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
Refs: nodejs/node-addon-api#832 Seems like FinalizeCallback needs to create a HandleScope since it calls ObjectWrap::~ObjectWrap() which might need to create a temporary Object handle here https://github.com/nodejs/node-addon-api/blob/master/napi-inl.h#L3558 at Realm (https://github.com/realm/realm-js) we have crashes with stacktrace at this location. While fixed in core for later versions we still need for older versions of Node.js PR-URL: nodejs/node-addon-api#832 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Kevin Eady <kevin.c.eady@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: NickNaso <nicoladelgobbo@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
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.
Seems like FinalizeCallback needs to create a HandleScope since it calls ObjectWrap::~ObjectWrap() which might need to create a temporary Object handle here https://github.com/nodejs/node-addon-api/blob/master/napi-inl.h#L3558
at Realm (https://github.com/realm/realm-js) we have crashes with stacktrace at this location.