Issue 34 in v8-juice: v8 regression causes crash of Object accessed post-destruction

[v8-j...@googlecode.com]

Status: SeemsToBeImpossible
Owner: sgb...@gmail.com
Labels: Type-Defect Priority-Critical

New issue 34 by sgb...@gmail.com: v8 regression causes crash of Object
accessed post-destruction
http://code.google.com/p/v8-juice/issues/detail?id=34

From a mail just sent to the v8 list:

-----------------------------
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?
--------------------------------


This might be limited to x64 (as yet unknown, but i've seen problems like
this before on x64 which don't show up on i32).

There is no way for us to fix this at the client level - either it must be
fixed in v8 or cvv8 client code must never access an Object after it is
destroyed via its WeakPtr callback (e.g. via ClassCreator::DestroyObject()).

[v8-j...@googlecode.com]

Comment #2 on issue 34 by sgb...@gmail.com: v8 regression causes crash of

Object accessed post-destruction
http://code.google.com/p/v8-juice/issues/detail?id=34

https://groups.google.com/forum/?fromgroups=#!topic/v8-dev/-O0FbmBqjOs

it seems that the cvv8 API will have to be majorly refactored to use
SetHiddenField() for storing the native and (optional) type-safety check
pointer. On Linux x64 and ia32 we're getting "pointer not aligned"
assertions on system-allocated pointers when using the new
Get/SetAlignedInternal...() (but not on Windows). This also breaks
v8-juice, but i'm not going to go back and fix all of that code - it will
have to either die off or be taken over by someone else.

<gripe>Sometimes i really hate the v8 devs. This type of thing is the
reason i stopped coding with Qt - continually having to go back and
fix/rework my apps when the devs change old behaviours.</gripe>

[v8-j...@googlecode.com]

Updates:
Status: Fixed

Comment #3 on issue 34 by sgb...@gmail.com: v8 regression causes crash of

Object accessed post-destruction
http://code.google.com/p/v8-juice/issues/detail?id=34

The post-destruction-deref crash is fixed (incidentally) in r2259. The
switch from Get/SetPointerInInternalField() to Get/SetInternalField(n,
External) (to work around broken alignment requirements) incidentally
worked around that problem.