Bug 186277

Summary: JSLock should clear last exception when releasing the lock
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, mark.lam, msaboff, saam, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch mark.lam: review+

Keith Miller
Reported 2018-06-04 12:42:50 PDT
JSLock should clear last exception when releasing the lock
Attachments
Patch (1.36 KB, patch)
2018-06-04 12:44 PDT, Keith Miller 		no flags
DetailsFormatted DiffDiff
Patch (1.40 KB, patch)
2018-06-04 14:37 PDT, Keith Miller 		mark.lam: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Keith Miller
Comment 1 2018-06-04 12:44:18 PDT
Created attachment 341909 [details] Patch
Mark Lam
Comment 2 2018-06-04 13:25:50 PDT
Comment on attachment 341909 [details] Patch r=me
Mark Lam
Comment 3 2018-06-04 13:27:01 PDT
Comment on attachment 341909 [details] Patch Wait a minute. What happens when we have a ReleaseAllLocks situation and want to GrabAllLocks again later?
Mark Lam
Comment 4 2018-06-04 13:29:29 PDT
(In reply to Mark Lam from comment #3) > Comment on attachment 341909 [details] > Patch > > Wait a minute. What happens when we have a ReleaseAllLocks situation and > want to GrabAllLocks again later? How about clearing lastException on unlocking JSLock only when vm.topCallFrame is null?
Keith Miller
Comment 5 2018-06-04 14:16:07 PDT
(In reply to Mark Lam from comment #3) > Comment on attachment 341909 [details] > Patch > > Wait a minute. What happens when we have a ReleaseAllLocks situation and > want to GrabAllLocks again later? DropAllLocks doesn't call willReleaseLock()... So it will have the some behavior it has now. willReleaseLock is only called from JSLock::unlock and only if the lock count is going to go to zero.
Keith Miller
Comment 6 2018-06-04 14:37:42 PDT
Created attachment 341919 [details] Patch
Keith Miller
Comment 7 2018-06-04 14:39:02 PDT
Nvm, you're right, we should only drop the last exception if we don't have a top call frame.
Keith Miller
Comment 8 2018-06-04 14:45:54 PDT
*** Bug 186221 has been marked as a duplicate of this bug. ***
Mark Lam
Comment 9 2018-06-04 14:46:40 PDT
Comment on attachment 341919 [details] Patch r=me
Keith Miller
Comment 10 2018-06-04 14:51:15 PDT
Committed r232482: <https://trac.webkit.org/changeset/232482>
Radar WebKit Bug Importer
Comment 11 2018-06-04 14:52:21 PDT
<rdar://problem/40782080>
Saam Barati
Comment 12 2018-06-11 18:58:38 PDT
Comment on attachment 341919 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341919&action=review > Source/JavaScriptCore/runtime/JSLock.cpp:203 > + if (!vm->topCallFrame) > + vm->clearLastException(); Don't we really just want this in ~VMEntryScope? Don't we drop locks and regrab them in interesting places. I feel like you're guarding against this with "!vm->topCallFrame", but that's exactly what ~VMEntryScope gives you.
Note You need to log in before you can comment on or make changes to this bug.