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

GC Crash With ObjectWrap + Exceptions #599

Closed
dananderson opened this issue Nov 12, 2019 · 0 comments
Closed

GC Crash With ObjectWrap + Exceptions #599

dananderson opened this issue Nov 12, 2019 · 0 comments
Assignees
@gabrielschulhof

Comments

@dananderson
Copy link

I am seeing crashes in the V8 GC when I have ObjectWraps that throw in subclass constructors. Example:

class MyObject : public ObjectWrap<MyObject> { MyObject(const CallbackInfo& info) : ObjectWrap<MyObject>(info) { throw Error::New(info.Env(), "something exceptional happened!"); } }

The ObjectWrap constructor will create a napi_ref for the object, associate this pointer and register a finalizer. If the subclass throws, the C++ runtime invalidates the this pointer. However, the pointer is still registered with the napi_ref. When the GC gets to the object, the finalizer tries to delete the invalid this and the system may crash.
@gabrielschulhof gabrielschulhof self-assigned this Nov 13, 2019
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Nov 13, 2019
objectwrap: gracefully handle constructor exceptions
12ddf49 
Move the `napi_wrap()` step out of the
`Napi::ObjectWrap<T>::ObjectWrap()` constructor to ensure that no
native instance pointer is associated with the JavaScript object under
construction if the native constructor should cause a JavaScript
exception to be thrown.

Fixes: nodejs#599
@gabrielschulhof gabrielschulhof mentioned this issue Nov 13, 2019
Closed
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Nov 14, 2019
objectwrap: gracefully handle constructor exceptions
9347e10 
Ensure that no native instance pointer is associated with the
JavaScript object under construction if the native constructor causes a
JavaScript exception to be thrown.

Fixes: nodejs#599
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Nov 15, 2019
objectwrap: gracefully handle constructor exceptions
f9a8178 
Ensure that no native instance pointer is associated with the
JavaScript object under construction if the native constructor causes a
JavaScript exception to be thrown.

Fixes: nodejs#599
@gabrielschulhof gabrielschulhof mentioned this issue Dec 25, 2019
Closed
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Dec 31, 2019
@blagoev
objectwrap: gracefully handle constructor exceptions
683bfcc 
Ensure that no native instance pointer is associated with the
JavaScript object under construction if the native constructor causes a
JavaScript exception to be thrown.

Two different cases must be taken into consideration:

1. The exception in the constructor was caused by
   `ObjectWrap<T>::ObjectWrap` when the call to `napi_wrap()` failed.
2. The exception in the constructor was caused by the constructor of
   the subclass of `ObjectWrap<T>` after `napi_wrap()` was already
   successful.

Fixes: nodejs#599
Co-authored-by: blagoev <[email protected]>
PR-URL: nodejs#600
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Jan 1, 2020
@blagoev
objectwrap: gracefully handle constructor exceptions
a1acd41 
Ensure that no native instance pointer is associated with the
JavaScript object under construction if the native constructor causes a
JavaScript exception to be thrown.

Two different cases must be taken into consideration:

1. The exception in the constructor was caused by
   `ObjectWrap<T>::ObjectWrap` when the call to `napi_wrap()` failed.
2. The exception in the constructor was caused by the constructor of
   the subclass of `ObjectWrap<T>` after `napi_wrap()` was already
   successful.

Fixes: nodejs#599
Co-authored-by: blagoev <[email protected]>
PR-URL: nodejs#600
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Jan 1, 2020
@blagoev
objectwrap: gracefully handle constructor exceptions
0cc1985 
Ensure that no native instance pointer is associated with the
JavaScript object under construction if the native constructor causes a
JavaScript exception to be thrown.

Two different cases must be taken into consideration:

1. The exception in the constructor was caused by
   `ObjectWrap<T>::ObjectWrap` when the call to `napi_wrap()` failed.
2. The exception in the constructor was caused by the constructor of
   the subclass of `ObjectWrap<T>` after `napi_wrap()` was already
   successful.

Fixes: nodejs#599
Co-authored-by: blagoev <[email protected]>
PR-URL: nodejs#600
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Jan 1, 2020
@blagoev
objectwrap: gracefully handle constructor exceptions
7ee9bfd 
Ensure that no native instance pointer is associated with the
JavaScript object under construction if the native constructor causes a
JavaScript exception to be thrown.

Two different cases must be taken into consideration:

1. The exception in the constructor was caused by
   `ObjectWrap<T>::ObjectWrap` when the call to `napi_wrap()` failed.
2. The exception in the constructor was caused by the constructor of
   the subclass of `ObjectWrap<T>` after `napi_wrap()` was already
   successful.

Fixes: nodejs#599
Co-authored-by: blagoev <[email protected]>
PR-URL: nodejs#600
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Jan 7, 2020
@blagoev
objectwrap: gracefully handle constructor exceptions
8cf3755 
Ensure that no native instance pointer is associated with the
JavaScript object under construction if the native constructor causes a
JavaScript exception to be thrown.

Two different cases must be taken into consideration:

1. The exception in the constructor was caused by
   `ObjectWrap<T>::ObjectWrap` when the call to `napi_wrap()` failed.
2. The exception in the constructor was caused by the constructor of
   the subclass of `ObjectWrap<T>` after `napi_wrap()` was already
   successful.

Fixes: nodejs#599
Co-authored-by: blagoev <[email protected]>
PR-URL: nodejs#600
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue May 11, 2020
@blagoev
objectwrap: gracefully handle constructor exceptions
663d627 
Ensure that no native instance pointer is associated with the
JavaScript object under construction if the native constructor causes a
JavaScript exception to be thrown.

Two different cases must be taken into consideration:

1. The exception in the constructor was caused by
   `ObjectWrap<T>::ObjectWrap` when the call to `napi_wrap()` failed.
2. The exception in the constructor was caused by the constructor of
   the subclass of `ObjectWrap<T>` after `napi_wrap()` was already
   successful.

Fixes: nodejs#599
Co-authored-by: blagoev <[email protected]>
PR-URL: nodejs#600
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Jun 12, 2020
@blagoev
objectwrap: gracefully handle constructor exceptions
1a51067 
Ensure that no native instance pointer is associated with the
JavaScript object under construction if the native constructor causes a
JavaScript exception to be thrown.

Two different cases must be taken into consideration:

1. The exception in the constructor was caused by
   `ObjectWrap<T>::ObjectWrap` when the call to `napi_wrap()` failed.
2. The exception in the constructor was caused by the constructor of
   the subclass of `ObjectWrap<T>` after `napi_wrap()` was already
   successful.

Fixes: nodejs#599
Co-authored-by: blagoev <[email protected]>
PR-URL: nodejs#600
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this issue Aug 24, 2022
@kevindavies8 @blagoev
objectwrap: gracefully handle constructor exceptions
85435af 
Ensure that no native instance pointer is associated with the
JavaScript object under construction if the native constructor causes a
JavaScript exception to be thrown.

Two different cases must be taken into consideration:

1. The exception in the constructor was caused by
   `ObjectWrap<T>::ObjectWrap` when the call to `napi_wrap()` failed.
2. The exception in the constructor was caused by the constructor of
   the subclass of `ObjectWrap<T>` after `napi_wrap()` was already
   successful.

Fixes: nodejs/node-addon-api#599
Co-authored-by: blagoev <[email protected]>
PR-URL: nodejs/node-addon-api#600
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this issue Aug 26, 2022
@Marlyfleitas @blagoev
objectwrap: gracefully handle constructor exceptions
af19fc1 
Ensure that no native instance pointer is associated with the
JavaScript object under construction if the native constructor causes a
JavaScript exception to be thrown.

Two different cases must be taken into consideration:

1. The exception in the constructor was caused by
   `ObjectWrap<T>::ObjectWrap` when the call to `napi_wrap()` failed.
2. The exception in the constructor was caused by the constructor of
   the subclass of `ObjectWrap<T>` after `napi_wrap()` was already
   successful.

Fixes: nodejs/node-addon-api#599
Co-authored-by: blagoev <[email protected]>
PR-URL: nodejs/node-addon-api#600
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this issue Sep 19, 2022
@wroy7860 @blagoev
objectwrap: gracefully handle constructor exceptions
59b433e 
Ensure that no native instance pointer is associated with the
JavaScript object under construction if the native constructor causes a
JavaScript exception to be thrown.

Two different cases must be taken into consideration:

1. The exception in the constructor was caused by
   `ObjectWrap<T>::ObjectWrap` when the call to `napi_wrap()` failed.
2. The exception in the constructor was caused by the constructor of
   the subclass of `ObjectWrap<T>` after `napi_wrap()` was already
   successful.

Fixes: nodejs/node-addon-api#599
Co-authored-by: blagoev <[email protected]>
PR-URL: nodejs/node-addon-api#600
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this issue Aug 11, 2023
@blagoev
objectwrap: gracefully handle constructor exceptions
226cbd8 
Ensure that no native instance pointer is associated with the
JavaScript object under construction if the native constructor causes a
JavaScript exception to be thrown.

Two different cases must be taken into consideration:

1. The exception in the constructor was caused by
   `ObjectWrap<T>::ObjectWrap` when the call to `napi_wrap()` failed.
2. The exception in the constructor was caused by the constructor of
   the subclass of `ObjectWrap<T>` after `napi_wrap()` was already
   successful.

Fixes: nodejs/node-addon-api#599
Co-authored-by: blagoev <[email protected]>
PR-URL: nodejs/node-addon-api#600
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gabrielschulhof gabrielschulhof

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

objectwrap: gracefully handle constructor exceptions gabrielschulhof/node-addon-api
2 participants
@dananderson @gabrielschulhof