1300454 - [meta] Intermittent mochitest timeout after tests in worker is finished

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Bevis Tseng[:bevis][:btseng](Exited)]

The clearAllDatabases() seems not finished after test in worker is done to which blocks the nextTestHarnessStep() to start the test again in main thread.

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

Hi Jan,

Is there any known issue in quota manager service recently that blocks this?

Comment 2

[Jan Varga [:janv]]

Hey Bevis, are you able to reproduce it locally, if so, how ?

Comment 3

[Bevis Tseng[:bevis][:btseng](Exited)]

no, I tried --repeat-until-failure --repeat 1000 for a single test, but I didn't encounter this problem yet. :|
BTW, there is some statistics in bug 1286210 since 2016-07-12 for reference.

Comment 4

[Bevis Tseng[:bevis][:btseng](Exited)]

Note:
According to the statistics so far, all the failure are only occurs in e10s and in linux build.

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

Root cause is found after keep adding logs to narrow down the issue. (This cannot be reproduced with locally with rr, unfortunately.)

In short, this is a regression of bug 1151017.

As explained in Bug 1151017 comment 16, 
We changed the logic for the database invalidation in parent from Abort(/* aForce */true) to Abort(false) and ensure that all the IDBTransaction in child will SendAbort() in IDBTransaction::OnRequestFinished() to parent to comply the flow defined in the spec of IDB v2.

However, we missed the one without any IDBRequest pending.

Hence, the solution is to SendAbort() to parent in IDBTransaction::AbortInternal() without checking |isInvalidated| if this IDBTransaction is still in INITIAL state.
http://searchfox.org/mozilla-central/rev/3411332d29d377bf86405fc3ea72461ea0cb0295/dom/indexedDB/IDBTransaction.cpp#642

In addition, if we check the 1st failure test cases, we can see that all these test cases are ended with an empty transaction without waiting txn.oncomplete is fired.

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

An experiment to identify the problem:
1. the original flow:
   https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b94938f620f&selectedJob=28458318
2. modified test_rename_index.js to finishTest() after txn.oncomplete is done.
   https://treeherder.mozilla.org/#/jobs?repo=try&revision=008b873fbf6c

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Patch: SendAbort() to parent after IDBTransaction is aborted in INITIAL state. r=janv

A simple fix for the regression as explained in comment 5.

This issue occurred frequently in linux debug build according to the OrangeFactor Robot reports in bug 1286210 and I always see about 10 failures in 100 runs when debugging, so I've triggered 100 runs for linux debug build in treeherder and wait for the test result complete:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=acfb09c831d6

Comment 8

[Jan Varga [:janv]]

I'll wait for try to finish, but the fix looks good to me.

Comment 9

[Jan Varga [:janv]]

Comment on attachment 8797439 [details] [diff] [review]
(v1) Patch: SendAbort() to parent after IDBTransaction is aborted in INITIAL state. r=janv

Review of attachment 8797439 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Please plan to request Aurora/Beta approval on this patch once it lands on mozilla-central. Thanks for fixing this!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6397682fabea
SendAbort() to parent after IDBTransaction is aborted in INITIAL state. r=janv

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6397682fabea

Comment 13

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8797439 [details] [diff] [review]
(v1) Patch: SendAbort() to parent after IDBTransaction is aborted in INITIAL state. r=janv

Approval Request Comment
[Feature/regressing bug #]:1151017
[User impact if declined]:This is a corner case that cause database connection won't be released if user try to *ForgetThisSite* while an *empty* transaction of the database is not committed:
1. It is not common use case to have an IDB transaction without any read/write operations.
2. The ForgetThisSite is not handy to the end user as described in https://support.mozilla.org/en-US/kb/delete-browsing-search-download-history-firefox#w_how-do-i-remove-a-single-website-from-my-history
[Describe test coverage new/current, TreeHerder]:
It's about 5/50 times of the failure without the patch in linux debug build but now it's all green after the patch is applied.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f00042ae1f242f346a10cbff9275f401a92fc51a&selectedJob=28575762&filter-searchStr=Linux%20debug%20Mochitest%20Mochitest%20M(3)
[Risks and why]: Risk is low, this patch ensures that the database connection will not be pending forever in this corner case but does not impact anything else.
[String/UUID change made/needed]:N/A

Comment 14

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8797439 [details] [diff] [review]
(v1) Patch: SendAbort() to parent after IDBTransaction is aborted in INITIAL state. r=janv

Approval Request Comment
[Feature/regressing bug #]:1151017
[User impact if declined]:This is a corner case that cause database connection won't be released if user try to *ForgetThisSite* while an *empty* transaction of the database is not committed:
1. It is not common use case to have an IDB transaction without any read/write operations.
2. The *ForgetThisSite* is not handy to the end user as described in https://support.mozilla.org/en-US/kb/delete-browsing-search-download-history-firefox#w_how-do-i-remove-a-single-website-from-my-history
[Describe test coverage new/current, TreeHerder]:
It's about 5/50 times of the failure without the patch in linux debug build but now it's all green after the patch is applied.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f5c943895853b36bc2348460da24a3a5763e5b11&filter-searchStr=Linux%20debug%20Mochitest%20e10s%20Mochitest%20e10s%20M-e10s(3)
[Risks and why]: Risk is low, this patch ensures that the database connection will not be pending forever in this corner case but does not impact anything else.
[String/UUID change made/needed]:N/A

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8797439 [details] [diff] [review]
(v1) Patch: SendAbort() to parent after IDBTransaction is aborted in INITIAL state. r=janv

Fixes an intermittent, Aurora51+, Beta50+

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/944f5e3f8f87

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/b35318b86b33