Bug 102283 – IndexedDB: Indexing tests are flaky-crashing

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

[Joshua Bell]

IndexedDB: Indexing tests are flaky-crashing

[Joshua Bell]

A couple of indexing related tests have started crashing:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=storage%2Findexeddb%2Fmozilla%2Fcursor-update-updates-indexes.html%2Cstorage%2Findexeddb%2Findex-duplicate-keypaths.html

Although some of the crashes predate it, http://wkrev.com/134529 is a likely candidate for the "spike" (hard to tell, there are so few of them).

While inspecting the code, I noticed this test 

if (!m_pendingEvents && isTaskQueueEmpty()) {
         // The last task event has completed and the task
         // queue is empty. Commit the transaction.
         commit();
}

This doesn't look at m_pendingPreemptiveEvents, so it could commit() early in the case of an indexing operation (the source of preemptive events), which could lead to calls being made into the IDBTransactionBackendImpl after it commit and was destroyed. Prior to http://wkrev.com/134529 the events would happen one at a time so the likelihood of this occurring would be lower.

[Joshua Bell]

Created attachment 174256 [details]
Patch

[Joshua Bell]

alecflett@ - please take a look?

[Joshua Bell]

+dglazkov@, +haraken@ as an FYI to the gardeners. This probably hasn't shown up on your radar yet, but if it does and this patch doesn't fix it please roll out http://wkrev.com/134529

[Alec Flett]

lgtm

[Joshua Bell]

tony@ - r?

[Tony Chang]

Comment on attachment 174256 [details]
Patch

Ok, let's try it.

[WebKit Review Bot]

Comment on attachment 174256 [details]
Patch

Clearing flags on attachment: 174256

Committed r134685: <http://trac.webkit.org/changeset/134685>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Joshua Bell]

Reopening - flakiness has not gone away.

The fix that was landed as http://wkrev.com/134685 is still believed to be a good addition so it should not be rolled out. It may explain a very low frequency of "early commits" that we have had reports of but have been unable to repro.

[Joshua Bell]

Created attachment 174498 [details]
Patch

[Joshua Bell]

tony@ - another r?

[Tony Chang]

Comment on attachment 174498 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=174498&action=review

> Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.cpp:251
> +    RefPtr<IDBTransactionBackendImpl> self(this);

Nit: It's common in WebKit code to name this type of self reference protect or protector (you can grep for other examples of this).

[Joshua Bell]

Created attachment 174506 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 174506 [details]
Patch for landing

Clearing flags on attachment: 174506

Committed r134838: <http://trac.webkit.org/changeset/134838>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Joshua Bell]

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