Bug 62128 - [Qt][V8] DumpRenderTree causes assertion in V8
Summary: [Qt][V8] DumpRenderTree causes assertion in V8
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Linux
: P2 Normal
Assignee: Peter Varga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-06 08:13 PDT by Peter Varga
Modified: 2011-06-07 01:23 PDT (History)
4 users (show)

See Also:


Attachments
proposed patch (2.17 KB, patch)
2011-06-06 08:28 PDT, Peter Varga
jedrzej.nowacki: review-
jedrzej.nowacki: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Varga 2011-06-06 08:13:54 PDT
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
#
Comment 1 Peter Varga 2011-06-06 08:28:56 PDT
Created attachment 96095 [details]
proposed patch
Comment 2 anton muhin 2011-06-06 08:50:41 PDT
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.
Comment 3 Peter Varga 2011-06-06 09:11:37 PDT
(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.
Comment 4 Simon Hausmann 2011-06-06 12:06:36 PDT
(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 :)
Comment 5 Peter Varga 2011-06-06 13:18:14 PDT
(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.
Comment 6 Jędrzej Nowacki 2011-06-07 01:20:08 PDT
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.
Comment 7 Jędrzej Nowacki 2011-06-07 01:23:38 PDT
Closing this bug as correct fix should be applied on the QtScript side.