Bug 60802 - Make GC validation more aggressive
: Make GC validation more aggressive
Status: RESOLVED FIXED
: WebKit
New Bugs
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
: 60881
:
  Show dependency treegraph
 
Reported: 2011-05-13 14:11 PST by
Modified: 2011-05-16 05:23 PST (History)


Attachments
Patch (75.16 KB, patch)
2011-05-13 14:17 PST, Oliver Hunt
no flags Review Patch | Details | Formatted Diff | Diff
Patch (75.25 KB, patch)
2011-05-13 15:31 PST, Oliver Hunt
no flags Review Patch | Details | Formatted Diff | Diff
crash log from debug Qt bot (3.94 KB, text/plain)
2011-05-14 01:09 PST, Csaba Osztrogonác
no flags Details


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-05-13 14:11:49 PST
Make GC validation more aggressive
------- Comment #1 From 2011-05-13 14:17:28 PST -------
Created an attachment (id=93509) [details]
Patch
------- Comment #2 From 2011-05-13 14:29:47 PST -------
(From update of attachment 93509 [details])
Attachment 93509 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/8698179
------- Comment #3 From 2011-05-13 14:33:27 PST -------
(From update of attachment 93509 [details])
Attachment 93509 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8695239
------- Comment #4 From 2011-05-13 15:31:53 PST -------
Created an attachment (id=93524) [details]
Patch
------- Comment #5 From 2011-05-13 15:42:05 PST -------
(From update of attachment 93524 [details])
r=me, assuming everything builds and doesn't crash
------- Comment #6 From 2011-05-13 16:07:37 PST -------
Committed r86469: <http://trac.webkit.org/changeset/86469>
------- Comment #7 From 2011-05-14 01:05:43 PST -------
(In reply to comment #5)
> (From update of attachment 93524 [details] [details])
> r=me, assuming everything builds and doesn't crash

(In reply to comment #6)
> Committed r86469: <http://trac.webkit.org/changeset/86469>

Unfortunately you didn't take Geoff's advice and caused hundreds crashes on Qt bot. Because of this I had to roll out: http://trac.webkit.org/changeset/86482

Reopen to fix it. I'll attach a crash log to help debugging.
------- Comment #8 From 2011-05-14 01:09:00 PST -------
Created an attachment (id=93550) [details]
crash log from debug Qt bot
------- Comment #9 From 2011-05-14 10:28:25 PST -------
(In reply to comment #7)
> (In reply to comment #5)
> > (From update of attachment 93524 [details] [details] [details])
> > r=me, assuming everything builds and doesn't crash
> 
> (In reply to comment #6)
> > Committed r86469: <http://trac.webkit.org/changeset/86469>
> 
> Unfortunately you didn't take Geoff's advice and caused hundreds crashes on Qt bot. Because of this I had to roll out: http://trac.webkit.org/changeset/86482

Thanks for the snarky comment.

> 
> Reopen to fix it. I'll attach a crash log to help debugging.
Crash log doesn't help because it's stripped of real symbol information.  We've had this problem with Qt before.  The build bots cannot strip symbols.
------- Comment #10 From 2011-05-14 10:30:42 PST -------
I'll be relanding this shortly on the assumption that the qt bot failures are due to errors in qt code rather than errors in the patch itself.  Given this patch had one purpose (find errors in code using jsc) the response to many errors should have been to determine what the errors were, not roll out the patch that identified them.
------- Comment #11 From 2011-05-14 10:35:59 PST -------
Lo and behold it _is_ an error in qt.

~QtRuntimeMethod removes itself from the QtInstance method cache during it's destructor, which is a point at which it is no longer guaranteed to be a valid object.

QtRuntimeMethod should use a finalizer to remove itself from the map not its destructor.
------- Comment #12 From 2011-05-14 12:54:13 PST -------
Thanks for the additional information. Please don't land it at the 
weekend, zherczeg will gladly fix the bug in Qt code path at monday 
morning (in european timezone).

Thanks for your patience.
------- Comment #13 From 2011-05-14 12:57:16 PST -------
(In reply to comment #12)
> Thanks for the additional information. Please don't land it at the 
> weekend, zherczeg will gladly fix the bug in Qt code path at monday 
> morning (in european timezone).
> 
> Thanks for your patience.

We actually need this patch in as we're seeing crashes due to gc bugs that this will expose.  Happily I've put up a patch at https://bugs.webkit.org/show_bug.cgi?id=60841 that should fix the issue.  Currently waiting on the EWS to tell me how to make it build properly.
------- Comment #14 From 2011-05-14 15:10:18 PST -------
Committed r86499: <http://trac.webkit.org/changeset/86499>