Skip to content

Conversation

petekanev
Copy link
Contributor

PR comprises of:

  • deprecate V8Extension GetInternalFieldCount and use V8 Object's InternalFieldCount API

  • fix crash after V8 GC was finished when incorrectly using the GetInternalFieldCount failed to resolve the JS Object

Should address crashes as found and described in nativescript-vue/nativescript-vue#32

…nalFieldCount API

fix crash after V8 GC was finished when incorrectly using the GetInternalFieldCount failed to resolve the JS Object
@ns-bot
Copy link

ns-bot commented Aug 9, 2017

💚

@petekanev petekanev requested a review from Plamen5kov August 9, 2017 10:11

bool ObjectManager::IsJsRuntimeObject(const v8::Local<v8::Object>& object) {
int internalFieldCount = NativeScriptExtension::GetInternalFieldCount(object);
int internalFieldCount = object->InternalFieldCount();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well spotted, great job!

/*
* Deprecated. Use V8 Object.InternalFieldCount() instead.
* TODO: Pete: remove in following 6.x update
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can almost bet my leg that no one's using it, but just to be safe.

Copy link
Contributor

@Plamen5kov Plamen5kov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thumbs up after a green build.

@petekanev petekanev merged commit 1b3cb97 into master Aug 9, 2017
@petekanev petekanev deleted the pete/fix-gc-crash branch August 9, 2017 11:06
@petekanev petekanev added this to the 3.2.0 milestone Aug 10, 2017
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.

3 participants