search

pmed / v8pp

Bind C++ functions and classes into V8 JavaScript engine
http://pmed.github.io/v8pp/
Other
901 stars 121 forks source link

Fixed externally referenced objects getting their destructor invoked #110

Closed mrsharpoblunto closed 5 years ago

mrsharpoblunto commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Hi @mrsharpoblunto

Since V8 version 7.5 IsNearDeath() member function in persistent handle class was removed. So in a591866066b682c6862ba5545b0e97d45b1a4a35 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 commented 5 years ago

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