Bug 196445 - [ Mac Debug ] REGRESSION (r242975) Layout Test storage/indexeddb/modern/deletedatabase-2-private.html is a flaky failure
Summary: [ Mac Debug ] REGRESSION (r242975) Layout Test storage/indexeddb/modern/delet...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-01 10:16 PDT by Shawn Roberts
Modified: 2019-06-07 18:30 PDT (History)
10 users (show)

See Also:


Attachments
Patch (3.12 KB, patch)
2019-06-07 11:11 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shawn Roberts 2019-04-01 10:16:03 PDT
The following layout test is flaky on Mac WK2 Debug

storage/indexeddb/modern/deletedatabase-2-private.html

Probable cause:

After changes made in https://trac.webkit.org/changeset/242975/webkit 

Test started to become a flaky failure on Mac Debug builds. It has shown up once on the iOS Simulator Release queue, and I could reproduce it locally, but only about 1 in 1000 iterations.

With Mac Debug it is failing on average 10 in 500 iterations, and I cannot get it to fail in r242974 or older.

Reproduced with:

run-webkit-tests storage/indexeddb/modern/deletedatabase-2-private.html --iterations 500 -f --debug

Flakiness Dashboard:

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=storage%2Findexeddb%2Fmodern%2Fdeletedatabase-2-private.html

Diff:

--- /Volumes/Data/slave/mojave-debug-tests-wk2/build/layout-test-results/storage/indexeddb/modern/deletedatabase-2-private-expected.txt
+++ /Volumes/Data/slave/mojave-debug-tests-wk2/build/layout-test-results/storage/indexeddb/modern/deletedatabase-2-private-actual.txt
@@ -11,9 +11,9 @@
 Requesting deleteDatabase
 First connection received versionchange event: oldVersion 1, newVersion null
 First version change complete
-Open request error: AbortError
 Delete database success: oldVersion 1, newVersion null
 Recreating database to make sure it's new and empty
+Open request error: AbortError
 Second upgrade old version - 0 new version - 1
 Unable to get object store in second upgrade transaction (which is correct because it should not be there)
 Second database upgrade success
Comment 1 Radar WebKit Bug Importer 2019-04-01 10:16:43 PDT
<rdar://problem/49483061>
Comment 2 Shawn Roberts 2019-04-01 10:23:05 PDT
Marked flaky in https://trac.webkit.org/changeset/243696/webkit while waiting for a fix.
Comment 3 Shawn Roberts 2019-05-14 15:56:06 PDT
When marking test as flaky in r243696 I did so on iOS, not on Mac.

Updated expectations properly in https://trac.webkit.org/changeset/245315/webkit
Comment 4 youenn fablet 2019-06-06 17:27:58 PDT
I debugged a bit.
From IPC we receive first from NetworkProcess to WebProcess:
1. A notification that a transaction is committed
2. A notification that a database is deleted
The order is always the same.

The first IPC message triggers synchronously the enqueuing of a complete event in IDBTransaction::fireOnComplete.
The second IPC message triggers synchronously the enqueuing of a success event in IDBOpenDBRequest::onDeleteDatabaseSuccess.

After the complete event is fired, a third event (an error event) is enqueued in IDBTransaction::dispatchEvent.

There is a race condition between the second event and the third event.
For this race to be visible, there is a need for the second IPC message to happen quickly enough after the first one so that the first event is not yet dispatched.

I guess that this is more the case with private browsing mode where deleting the database is in memory so very quick and does not require hopping to background threads.
This also explains why this flakiness happens more on debug mode where the enqueueing might be slower.


It is not clear to me whether the spec mandates the order.
I guess we could rewrite the test to unflake it.
Comment 5 youenn fablet 2019-06-06 17:46:11 PDT
Some other observations:
- I failed running the test on Chrome (timeout) and Firefox (failing assertions)
- https://w3c.github.io/IndexedDB/#commit-a-transaction mentions enqueuing a task to fire a complete event. There is no description of enqueuing a request error/success event just after dispatching a complete event.
Comment 6 youenn fablet 2019-06-07 11:11:00 PDT
Created attachment 371601 [details]
Patch
Comment 7 WebKit Commit Bot 2019-06-07 18:30:52 PDT
Comment on attachment 371601 [details]
Patch

Clearing flags on attachment: 371601

Committed r246227: <https://trac.webkit.org/changeset/246227>
Comment 8 WebKit Commit Bot 2019-06-07 18:30:53 PDT
All reviewed patches have been landed.  Closing bug.