Bug 186277 – JSLock should clear last exception when releasing the lock

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Keith Miller]

JSLock should clear last exception when releasing the lock

[Keith Miller]

Created attachment 341909 [details]
Patch

[Mark Lam]

Comment on attachment 341909 [details]
Patch

r=me

[Mark Lam]

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]

(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]

(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]

Created attachment 341919 [details]
Patch

[Keith Miller]

Nvm, you're right, we should only drop the last exception if we don't have a top call frame.

[Keith Miller]

*** Bug 186221 has been marked as a duplicate of this bug. ***

[Mark Lam]

Comment on attachment 341919 [details]
Patch

r=me

[Keith Miller]

Committed r232482: <https://trac.webkit.org/changeset/232482>

[Radar WebKit Bug Importer]

<rdar://problem/40782080>

[Saam Barati]

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.