Bug 128537

Summary: Dedicated worker crash caused by global DFG worklists + GC
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, barraclough, commit-queue, fpizlo, ggaren, mark.lam, mhahnenberg, mmirman, msaboff, oliver, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch none

Attachments
Patch (7.18 KB, patch)
2014-02-18 18:31 PST, Mark Hahnenberg
no flags
Filip Pizlo
Comment 1 2014-02-10 09:26:46 PST
This allegedly began with: http://trac.webkit.org/log/?verbose=on&rev=163691&stop_rev=163691 But the assertion failure is: ASSERTION FAILED: m_totalBytesCopied <= m_storageSpace.size() This implies a long-standing GC copying bug.
Filip Pizlo
Comment 2 2014-02-10 09:32:47 PST
Skipped in r163791
Alexey Proskuryakov
Comment 3 2014-02-11 22:42:45 PST
Mark Hahnenberg
Comment 4 2014-02-18 17:11:43 PST
(In reply to comment #1) > This allegedly began with: > > http://trac.webkit.org/log/?verbose=on&rev=163691&stop_rev=163691 > > But the assertion failure is: > > ASSERTION FAILED: m_totalBytesCopied <= m_storageSpace.size() > > This implies a long-standing GC copying bug. After some investigation, it appears that the process-global DFG worklists are causing objects to participate in the garbage collections of VMs other than the one they were allocated in. This started manifesting in the worker tests because they're one of the few WebKit tests that do multithreaded JS.
Filip Pizlo
Comment 5 2014-02-18 17:15:04 PST
(In reply to comment #4) > (In reply to comment #1) > > This allegedly began with: > > > > http://trac.webkit.org/log/?verbose=on&rev=163691&stop_rev=163691 > > > > But the assertion failure is: > > > > ASSERTION FAILED: m_totalBytesCopied <= m_storageSpace.size() > > > > This implies a long-standing GC copying bug. > > After some investigation, it appears that the process-global DFG worklists are causing objects to participate in the garbage collections of VMs other than the one they were allocated in. This started manifesting in the worker tests because they're one of the few WebKit tests that do multithreaded JS. Whoa! That's pretty awesome. So, the fix is just for the following methods: bool isActive() const { return !!m_plans.size(); } void visitChildren(SlotVisitor&, CodeBlockSet&); // Only called on the main thread after suspending all threads. to filter based on VM. That implies passing the VM& in each of them. That also means that: void suspendAllThreads(); void resumeAllThreads(); need to be smart enough to not allow "double suspension" of threads. Or to handle it gracefully. You could accomplish that super easily by having an internal Mutex that suspendAllThreads() locks before doing anything else, and resumeAllThreads() unlocks after.
Mark Hahnenberg
Comment 6 2014-02-18 18:31:19 PST
WebKit Commit Bot
Comment 7 2014-02-19 10:36:10 PST
Comment on attachment 224574 [details] Patch Clearing flags on attachment: 224574 Committed r164371: <http://trac.webkit.org/changeset/164371>
WebKit Commit Bot
Comment 8 2014-02-19 10:36:13 PST
All reviewed patches have been landed. Closing bug.
Mark Hahnenberg
Comment 9 2014-02-19 11:09:53 PST
*** Bug 128745 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.