Bug 132088 – The GC should only resume compiler threads that it suspended in the same GC pass

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

Currently this scenario can occur:
1. Thread 1 starts a GC and tries to suspend DFG worklist threads.  However, no worklists were created yet at the that time.
2. Thread 2 starts to compile some functions and creates a DFG worklist, and acquires the worklist thread's lock.
3. Thread 1's GC completes and tries to resume suspended DFG worklist thread.  This time, it sees the worklist created by Thread 2 and ends up unlocking the worklist thread's lock that is supposedly held by Thread 2.

Thereafter, chaos ensues.

The fix is to cache the worklists that were actually suspended by each GC pass, and only resume those when the GC is done.

[Radar WebKit Bug Importer]

<rdar://problem/16706259>

[Mark Lam]

Created attachment 230022 [details]
the patch

[Mark Hahnenberg]

Comment on attachment 230022 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230022&action=review

r=me

> Source/JavaScriptCore/heap/Heap.cpp:1025
> +    ASSERT(!m_suspendedCompilerWorklists.size());

I find it's more readable to say "m_suspendedCompilerWorklists.isEmpty()" instead.

> Source/JavaScriptCore/heap/Heap.cpp:1235
> +    for (auto worklist : m_suspendedCompilerWorklists)
> +        worklist->resumeAllThreads();
> +    m_suspendedCompilerWorklists.clear();

You should change the other places in the GC to only iterate this list. If the GC itself didn't suspend a work list then it shouldn't be touching that work list at all.

[Mark Lam]

Thanks.  Suggested changes applied.  Landed in r167733: <http://trac.webkit.org/r167733>.