[Webkit-unassigned] [Bug 128766] Web Inspector: CRASH when evaluating in console of JSContext RWI with disabled breakpoints

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=128766


Mark Lam <mark.lam at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|webkit-unassigned at lists.web |mark.lam at apple.com
                   |kit.org                     |
                 CC|                            |barraclough at apple.com,
                   |                            |fpizlo at apple.com,
                   |                            |ggaren at apple.com,
                   |                            |mario at webkit.org,
                   |                            |mhahnenberg at apple.com,
                   |                            |mmirman at apple.com,
                   |                            |oliver at apple.com




--- Comment #3 from Mark Lam <mark.lam at apple.com>  2014-02-22 12:22:34 PST ---
This issue is because we now allow more than 1 JS threads to drop the VM lock.  Consider the following scenario:

1. Thread T1 locks the VM and enters the VM to execute some JS code.
2. On entry, T1 detects that VM::m_entryScope is null i.e. this is the first time it entered the VM.
    T1 sets VM::m_entryScope to T1's entryScope.
3. T1 drops all locks.

4. Thread T2 locks the VM and enters the VM to execute some JS code.
    On entry, T2 sees that VM::m_entryScope is NOT null, and therefore does not set the entryScope.
5. T2 drops all locks.

6. T1 re-grabs locks.
7. T1 returns all the way out of JS code.  On exit from the outer most JS function, T1 clears VM::m_entryScope (because T1 was the one who set it).
8. T1 unlocks the VM.

9. T2 re-grabs locks.
10. T2 proceeds to execute some code and expects VM::m_entryScope to be NOT null, but it turns out to be null.  Assertion failures and crashes ensue.

I think the proper fix for each thread to set its own VMEntryScope and hence, JSGlobalObject at VM entry.  Since we're not requiring different threads to be synchronized on the order in which locks are re-grabbed after being dropped, the act of switching JS execution from T1 to T2 can be thought of conceptually as a thread context switch (T1 and T2's stacks are not synchronized), whereas previously, it was like an RPC call (T1 and T2's stacks are synchronized at the point of the VM lock changing hands).  As such, both T1 and T2 should have their own VMEntryScope just like they would have their own per-thread context.

What this means, in terms of implementation of the fix, is:

1. JSLock::dropAllLocks() saves VM::m_entryScope in thread local data, and sets VM::m_entryScope to null.
2. JSLock::grabAllLocks() sets VM::m_entryScope to the saved entryScope in thread local data.

After we dropped the locks, if a second thread enters the VM for the first time, it will see a null VM::m_entryScope and install its own as expected.  If the second thread unlocks the VM (either because it has exited out of the VM altogether or it dropped the locks), the original thread can re-grab the lock and re-install its own entryScope and continue executing as expected.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.