| Summary: | IndexedDB: "ASSERTION FAILED: !m_requestFinished" hit in IDBRequest::dispatchEvent | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joshua Bell <jsbell> | ||||||||||
| Component: | WebCore Misc. | Assignee: | Joshua Bell <jsbell> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | alecflett, dgrogan, tony, webkit.review.bot | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Bug Depends on: | 90412 | ||||||||||||
| Bug Blocks: | |||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 142804 [details]
Real repro case
Comment on attachment 142799 [details]
Repro case
Whoops. First repro had been reduced too far and I was testing the wrong file. Look at the second one that has noSuchFunction
(In reply to comment #2) > (From update of attachment 142799 [details]) > Whoops. First repro had been reduced too far and I was testing the wrong file. Look at the second one that has noSuchFunction ... and I admit I now have no idea what's going on. Changing the ordering of lines in the minimal repro - e.g. when the non-existent handler is assigned - makes the issue go away, which means my previous theory is incorrect. Ah, here's the key bit:
request = store.openCursor();
request.onsuccess = function () {
event.target.result.continue();
throw "foo";
};
If an exception is thrown in the event handler after the continue() is called the ASSERT is hit.
The uncaught exception is causing the transaction to be aborted, which closes the cursor which sets m_requestFinished on the IDBRequest between an event being enqueued and it being dispatched. (In reply to comment #5) > The uncaught exception is causing the transaction to be aborted, which closes the cursor which sets m_requestFinished on the IDBRequest between an event being enqueued and it being dispatched. Scratch that, not quite correct. Here's what's going on: * success event fired at the cursor request * success event dispatched by the cursor request * script calls continue(), which queues an task in the transaction * script raises an exception, which aborts the transaction * transaction aborting closes the cursor * transaction aborting aborts the queued task, which fires an error event at the cursor request * cursor request attempt to dispatch the error event, and ASSERTs Seems like we need to untangle things slightly - either the cursor shouldn't "finish" the request while there are outstanding events, or we should allow the error event to be dispatched even if the cursor is closed. Given that dispatch can be arbitrarily delayed, it seems like the latter is more tractable. (The timing difference between event queuing and dispatching is tricksy.) Note that this still repros after the untangling in 90412 (which makes sense because that patch should only impact multiprocess ports, whereas this repros DRT), but any fix here should wait until that lands since it touches related code. Created attachment 153361 [details]
Patch
Counting only "interesting" LOC, IDBRequest.cpp is now 20% ASSERTs! dgrogan@, alecflett@ - please take a look. Comment on attachment 153361 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153361&action=review LGTM > Source/WebCore/Modules/indexeddb/IDBRequest.h:76 > bool resetReadyState(IDBTransaction*); Can this line be deleted? Created attachment 153605 [details]
Patch
(In reply to comment #11) > > Source/WebCore/Modules/indexeddb/IDBRequest.h:76 > > bool resetReadyState(IDBTransaction*); > > Can this line be deleted? Good catch - deleted! tony@ - r? Comment on attachment 153605 [details] Patch Clearing flags on attachment: 153605 Committed r123275: <http://trac.webkit.org/changeset/123275> All reviewed patches have been landed. Closing bug. |
Created attachment 142799 [details] Repro case Seen in DumpRenderTree with attached LayoutTest (which could be dropped into storage/indexeddb for dependencies): crash log for DumpRenderTree (pid 8867): STDERR: ASSERTION FAILED: !m_requestFinished STDERR: ../../third_party/WebKit/Source/WebCore/Modules/indexeddb/IDBRequest.cpp(342) : virtual bool WebCore::IDBRequest::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) What's probably happening is the transaction is completing, and IDBTransaction::onComplete() calls IDBTransaction::closeOpenCursors() calls IDBCursor::close() calls IDBRequest::finishCursor() which sets m_cursorFinished, which allows m_requestFinished to be set. This may be happening too early.