Bug 128447 - JSLock should not "restore" stack limit values if it did not re-grab locks
Summary: JSLock should not "restore" stack limit values if it did not re-grab locks
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-07 23:32 PST by Mark Lam
Modified: 2014-02-08 00:23 PST (History)
7 users (show)

See Also:

Attachments
the patch. (5.07 KB, patch)
2014-02-07 23:44 PST, Mark Lam 		ggaren: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2014-02-07 23:32:19 PST 
In the existing code, if DropAllLocks is instantiate with DontAlwaysDropLocks in a thread that does not own the JSLock, then a bug will manifest where:
1. The DropAllLocks constructor will save the VM's stackPointerAtEntry, lastStackTop, and reservedZoneSize even though it will not drop the JSLock.
2. The DropAllLocks destructor will restore those 3 values to the VM even though the JSLock will not grab its internal lock for this thread.

The former only causes busy work but does not impact correctness.  The latter however, will corrupt those 3 VM values which belong to the thread that actually owns the JSLock.

The fix is to only save the values when  the JSLock will actually drop its internal lock, and only restore the values if it did re-grab the internal lock.
Comment 1 Mark Lam 2014-02-07 23:44:12 PST 
Created attachment 223554 [details]
the patch.
Comment 2 Geoffrey Garen 2014-02-08 00:07:00 PST 
Comment on attachment 223554 [details]
the patch.

r=me
Comment 3 Mark Lam 2014-02-08 00:19:19 PST 
Comment on attachment 223554 [details]
the patch.

I'll land the patch manually so that I can move on to the next patch.
Comment 4 Mark Lam 2014-02-08 00:23:10 PST 
Thanks.  Landed in r163700: <http://trac.webkit.org/r163700>.