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

Fixed externally referenced objects getting their destructor invoked #110

Closed
wants to merge 7 commits into from

Conversation

mrsharpoblunto
Copy link
Contributor

I noticed an issue where an object wrapped via v8pp::reference_external was having its destructor called after the wrapper was garbage collected. This caused crashes due to double freeing in my app because I was also calling the object destructor.

This seems to be because in the SetWeak callback, all objects get their destructor called, regardless of the call_dtor param in the wrap_object method. This PR queries the internal call_dtor field and only calls the destructor if the wrap_object method specified that this should be the case.

@mrsharpoblunto
Copy link
Contributor Author

Whoops, didn't realize that v8::WeakCallbackType::kInternalFields only passes the first 2 internal fields to the callback - so the destructor never gets called. I'll rethink how this should work. Just wondering would the reference/unreference_external api's be necessary if a new unowned_ptr trait was added instead that allowed storing a raw pointer but never called the objects destructor?

@pmed
Copy link
Owner

pmed commented Jun 5, 2019

Hi Glenn,

Thank you for the contribution.

Could you please supply a test case that demonstrates the issue? As far as I remember, there was a similar issue #60, and I tried to fix it by placing the call_dtor flag into the 3rd JS object field.

Currently this call_dor is being extracted in reset_object() only when the object's persistent handle IsNearToDeath(). Maybe this flag should be extracted always.

So we only need a single bit, in order to mark the object for deletion with its destructor. Such a flag could also be stored together with Global handle in object_registry::objects_ mapping.

@pmed
Copy link
Owner

pmed commented Jun 23, 2019

Hi @mrsharpoblunto

Since V8 version 7.5 IsNearDeath() member function in persistent handle class was removed. So in a591866 I store the call_dtor bool flag along with the weak persistent handle for a wrapped C++ object in a wrapped_object struct.

Please check the issue is gone on your side after this fix. I could close this pull request, if it would be fine.

@mrsharpoblunto
Copy link
Contributor Author

Hi @pmed everything working with your latest changes. Thanks!

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

Successfully merging this pull request may close these issues.

2 participants