Fwd: Painful regression in Object destruction behaviour

[Stephan Beal]

FYI... this v8 change breaks certain aspects of v8-juice/cvv8 (as in "crashes the app")...

---------- Forwarded message ----------
From: Stephan Beal <sgb...@googlemail.com>
Date: Sat, Dec 22, 2012 at 2:27 PM
Subject: Painful regression in Object destruction behaviour
To: v8-u...@googlegroups.com


Hi, devs,

i have just found a regression (from "sometime since last summer") in the handling of Object destruction. Consider this JS code:

    assert(m.destroy(), 'm.destroy()');

    assertThrows( function() {m.doFoo();} );

(m is a bound Native object)

m.destroy() simply triggers the WeakPtr destructor callback and Dispose()s the handle. What USED to happen after that was that any method calls could still be made on m (like the call to m.doFoo() above) but my framework would see that the Native binding in that Object (i.e. an Internal Field) was missing and would trigger a JS-side exception. What now happens is that the call to m.doFoo() triggers a v8 assertion as a side-effect of my code trying to figure out of m still points to a valid native. So m.doFoo() now results in the following assertion:

ConvertDemo.hpp:246:~BoundSubNative(): @0x1a47880 is destructing

ConvertDemo.hpp:53:~BoundNative(): @0x1a47880 is destructing.

Assertion OK: m.destroy()

#

# Fatal error in ../src/objects-inl.h, line 2386

# CHECK(object->IsJSObject()) failed

#

==== C stack trace ===============================

 1: V8_Fatal

 2: v8::internal::JSObject::cast(v8::internal::Object*)

 3: ??

 4: v8::Object::GetPointerFromInternalField(int)

 5: ??

 6: ??

 7: ??

 8: ??

 9: cvv8::Detail::MethodToInCaVoid<BoundNative, void (), &(BoundNative::doFoo()), true>::Call(v8::Arguments const&)

10: ??

11: ??

12: ??

13: ??

i don't know if this change was intentional but it hoses an inordinate amount of my code which assert()s that my framework can correctly avoid stepping on a deallocated native pointer (after the WeakPtr destruction callback is called on it). It also makes it painfully simple to crash the native app from script code, the avoidance of which is the whole purpose of my framework catching post-destruction access to deallocated natives.

(Yes, i know that GetPointerFromInternalField() is deprecated but GetAlignedPointerFromInternalField() asserts with not-aligned errors on x64!)

Is this a bug or a feature which i need to go rewire my code to deal with?

--
----- stephan beal
http://wanderinghorse.net/home/stephan/

http://gplus.to/sgbeal

--
----- stephan beal
http://wanderinghorse.net/home/stephan/

http://gplus.to/sgbeal

[Anton Yemelyanov]

Can GetAlignedPointerFromInternalField() be adopted to use a wrapper object that is always aligned?

[Stephan Beal]

On Sat, Dec 22, 2012 at 5:33 PM, Anton Yemelyanov <anton.ye...@gmail.com> wrote:

Can GetAlignedPointerFromInternalField() be adopted to use a wrapper object that is always aligned?

NO idea. i'm using the system allocator (new T()) and expect everything to be aligned automagically. i suspect this is an x64-specific bug (Badger is not seeing it on i32). i'll wait and see what the v8 devs have to say about it. For now i've locally switched back to the deprecated GetPointerFromInternalField(), but that function has a regression which asserts in v8 on some of my tests. :/

[Stephan Beal]

On Sat, Dec 22, 2012 at 7:12 PM, Stephan Beal <sgb...@googlemail.com> wrote:

On Sat, Dec 22, 2012 at 5:33 PM, Anton Yemelyanov <anton.ye...@gmail.com> wrote:

Can GetAlignedPointerFromInternalField() be adopted to use a wrapper object that is always aligned?

NO idea. i'm using the system allocator (new T()) and expect everything to be aligned automagically. i suspect this is an x64-specific bug (Badger is not seeing it on i32). i'll wait and see what the v8 devs have to say about it. For now i've locally switched back to the deprecated GetPointerFromInternalField(), but that function has a regression which asserts in v8 on some of my tests. :/

FYI: this has been resolved. We ported from Get/Set[Aligned]PointerInInternalField() to Get/SetInternalField() using an explicit External wrapper for the void pointer, and that works just fine (i _thought_ that the other versions were just thin wrappers around that, but they now behave differently). Badger has confirmed that this change works on MSVC 2008.