JSLock could have an imbalanced and unexpected change to the global thread data's AtomicStringTable. The expected balance between JSLock::didAcquireLock and JSLock::willReleaseLock wouldn't happen if the VM was not set (during destruction of a VM).
Created attachment 235264 [details] [PATCH] Proposed Fix I have been trying to create a test for this but it is proving difficult. My plan is to create a JSContext on a non-main thread (so a non-main AtomicStringTable), delete the JSContext on the main thread (such that JSLock would have unbalanced lock and release leaving the wrong AtomicStringTable). But it seems I'm missing some complexity. In any case, the reproducible case I have (a larger application) reproduced the issue reliably.
<rdar://problem/17041912>
Comment on attachment 235264 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=235264&action=review Patch looks fine to me but would be good if Geoff took a look as well. > Source/JavaScriptCore/runtime/JSLock.cpp:170 > + if (m_vm) { Early return?
r171367: <http://trac.webkit.org/changeset/171367>
Re-opened since this is blocked by bug 135192
Stealing ap's message from the wrong bug: "This change broke three API tests, so I'm going to roll out. Tests that failed: WKUserContentController.ScriptMessageHandlerWithNavigation WebKit2.RestoreSessionStateContainingFormData Tests that timed out: DeviceScaleFactorOnBack.WebKit2" The UI process is stuck in a HashTable lock adding a string to the AtomicString table from inside decidePolicyForResponse
(In reply to comment #6) > Stealing ap's message from the wrong bug: > > "This change broke three API tests, so I'm going to roll out. > > Tests that failed: > WKUserContentController.ScriptMessageHandlerWithNavigation > WebKit2.RestoreSessionStateContainingFormData > Tests that timed out: > DeviceScaleFactorOnBack.WebKit2" I can reproduce. Thinking about things, now we might incorrectly balance in the opposite way after this change. We need to handle what will happen if: - we acquire the lock with a vm (set the atomic string table) - m_vm gets cleared - we release the lock without a vm => we need to restore the atomic string table. Making the appropriate changes the tests all pass. I need to test the original case and then I'll put up another patch.
Created attachment 235376 [details] [PATCH] Proposed Fix
Comment on attachment 235376 [details] [PATCH] Proposed Fix What if we set the table on entry, and it was null on entry? How will it get back to null?
(In reply to comment #9) > (From update of attachment 235376 [details]) > What if we set the table on entry, and it was null on entry? How will it get back to null? Good point. My understanding is that the WTFThreadData's AtomicStringTable can (and should) never be null. I could ASSERT that now, and perhaps ultimately we could move to references. Alternatively we could have a bool alongside the pointer.
(In reply to comment #10) > (In reply to comment #9) > > (From update of attachment 235376 [details] [details]) > > What if we set the table on entry, and it was null on entry? How will it get back to null? > > Good point. My understanding is that the WTFThreadData's AtomicStringTable can (and should) never be null. I could ASSERT that now, and perhaps ultimately we could move to references. Alternatively we could have a bool alongside the pointer. It looks like WTFThreadData always sets a default table, so I think you're right. Let's ASSERT.
Created attachment 235474 [details] [PATCH] For Landing Added ASSERTs. Re-ran my tests without a hitch.
r171558: <http://trac.webkit.org/changeset/171558>