Bug 62128 – [Qt][V8] DumpRenderTree causes assertion in V8

• Component: WebCore Misc.
• Status: RESOLVED INVALID

[Peter Varga]

During the destruction of QScriptEngine the QScriptOriginalObject's destroy() function causes assertion in V8 in debug mode:

#
# Fatal error in ../../3rdparty/v8/src/api.h, line 555
# CHECK((blocks_.is_empty() && prev_limit == __null) || (!blocks_.is_empty() && prev_limit != __null)) failed
#

[Peter Varga]

Created attachment 96095 [details]
proposed patch

[anton muhin]

Comment on attachment 96095 [details]
proposed patch

May you elaborate a bit?  If QSE needs a handle which should survive w/o handle scope, it usually should use persistent handle inside.  I've got a feeling that it still uses local handle which is defined in some handle scope which just happens to live long enough to allow DRT to run normally.

[Peter Varga]

(In reply to comment #2)
> (From update of attachment 96095 [details])
> May you elaborate a bit?  If QSE needs a handle which should survive w/o handle scope, it usually should use persistent handle inside.  I've got a feeling that it still uses local handle which is defined in some handle scope which just happens to live long enough to allow DRT to run normally.

Actually, you are right. QtScript has an own global object (QScriptOriginalGlobalObject) which uses Local handles instead of Persistent. It was an optimization to make the creation of QSOGO faster. This is the reason why we need to avoid the instantiation of QSE inside a HandleScope.

[Simon Hausmann]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 96095 [details] [details])
> > May you elaborate a bit?  If QSE needs a handle which should survive w/o handle scope, it usually should use persistent handle inside.  I've got a feeling that it still uses local handle which is defined in some handle scope which just happens to live long enough to allow DRT to run normally.
> 
> Actually, you are right. QtScript has an own global object (QScriptOriginalGlobalObject) which uses Local handles instead of Persistent. It was an optimization to make the creation of QSOGO faster. This is the reason why we need to avoid the instantiation of QSE inside a HandleScope.

Hm, if QSOGO uses local handles, why doesn't it (or its caller?) use its own HandleScope?

Differently asked: Why does QtScript rely on an externally defined handle scope instead of defining its own? (I must be missing something, sorry if my question sounds stupid :)

[Peter Varga]

(In reply to comment #4)

> Hm, if QSOGO uses local handles, why doesn't it (or its caller?) use its own HandleScope?
>
> Differently asked: Why does QtScript rely on an externally defined handle scope instead of defining its own? (I must be missing something, sorry if my question sounds stupid :)

QSOGO has own HandleScope but this HandleScope is removed when the externally defined HandleScope ceased.

[Jędrzej Nowacki]

Comment on attachment 96095 [details]
proposed patch

The optimization in QtScript is wrong and it should be removed. Assumption that I made that QtScript is controlling exclusively V8 API is not more valid.

The patch it doesn't solve the real problem here. Still, destruction order of HandleScopes is wrong if QScriptEngine doesn't contain the topmost HandleScope. It just happened to work :(.

Please look at description of QSOGO class, I'd love to kill the class. It was purely created because of missing V8 C++ API, (we are "cheating" on JS side there :-) ). I do think that we should focus on removing the class instead of aggressive optimization.

[Jędrzej Nowacki]

Closing this bug as correct fix should be applied on the QtScript side.