1245615 - Intermittent browser_storage_dynamic_windows.js | A promise chain failed to handle a rejection: - at resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/storage.js:1025 - TypeError: this.hostVsStores is null

Status: RESOLVED FIXED

(DevTools :: Storage Inspector, defect)

Description

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

https://treeherder.mozilla.org/logviewer.html#?job_id=7021087&repo=fx-team


Started to appear when bug 1003860 landed.

Comment 1

[Intermittent Failures Robot]

29 automation job failures were associated with this bug yesterday.

Repository breakdown:
* fx-team: 25
* mozilla-inbound: 4

Platform breakdown:
* windows8-64: 25
* windows7-32: 4

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1245615&startday=2016-02-03&endday=2016-02-03&tree=all

Comment 2

[Alexandre Poirot [:ochameau]]

I'll look into this. I thought I got rid of this one. I've seen it appear is some try runs of bug 1003860, but seemed to be fixed with additional changes.

Comment 3

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=deed5d09d930

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: patch v1

Nullify hostVsStores only after event unregistration to prevent exceptions.

Hum. I may have fixed one intermittent before landing bug 1003860.
But it looks like there is another similar one.

I already fixed the scenario where onWindowReady was called *early*
and hostVsStores wasn't already set. (by setting hostVsStores in prepopulate function)
But it looks like onWindowReady may also be called late, after hostVsStores is nullified.

Comment 5

[Intermittent Failures Robot]

60 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* fx-team: 36
* mozilla-inbound: 19
* mozilla-central: 4
* try: 1

Platform breakdown:
* windows8-64: 44
* windows7-32: 16

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1245615&startday=2016-02-01&endday=2016-02-07&tree=all

Comment 6

[Alexandre Poirot [:ochameau]]

Comment on attachment 8715750 [details] [diff] [review]
patch v1

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

Looks like it does fix this intermittent, per last try push.
See comment 4 about this modification.

Comment 7

[Intermittent Failures Robot]

17 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 13
* mozilla-central: 2
* try: 1
* fx-team: 1

Platform breakdown:
* windows8-64: 15
* windows7-32: 1
* linux64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1245615&startday=2016-02-08&endday=2016-02-08&tree=all

Comment 8

[Alexandre Poirot [:ochameau]]

https://hg.mozilla.org/integration/fx-team/rev/0d49792ec539b466c2489718ba785c202036cdef
Bug 1245615 - Nullify hostVsStores only after event unregistration to prevent exceptions. r=mratcliffe

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0d49792ec539

Comment 10

[Phil Ringnalda (:philor)]

Alas, not actually fixed.

Comment 11

[Intermittent Failures Robot]

62 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 40
* fx-team: 12
* mozilla-central: 8
* try: 2

Platform breakdown:
* windows8-64: 37
* windows7-32: 18
* linux64: 7

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1245615&startday=2016-02-08&endday=2016-02-14&tree=all

Comment 12

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=daf1e8c80a80

Comment 13

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c6a0e556a140

Comment 14

[Alexandre Poirot [:ochameau]]

Attachment: Just clear hostVsStores map as some async code may still use them.

Ok. I think that's it.
populateStoresForHost is async and yield on "caches.open()".
I can easily imagine that it last long enough to finish after destroy() is called...

So instead of nullifying the maps, just clear them.
We are going to have some left over if the test mix up things,
but at this we won't have this race exception...

Comment 15

[Alexandre Poirot [:ochameau]]

Comment on attachment 8719808 [details] [diff] [review]
Just clear hostVsStores map as some async code may still use them.

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

Wasn't able to reproduce it on try.
Just got one failure, but on indexeddb:
  TypeError: this.storageActor is null
This is a similar issue, where onWindowReady most likely also last long enough to overlap call to destroy.
But I don't want to keep reference to storageActor.
It would be better for this one to ensure the test to wait for some additional event,
but I don't know which one. The test already waits for storage-update, which, I imagine is fired after the window-ready?

Comment 16

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8719808 [details] [diff] [review]
Just clear hostVsStores map as some async code may still use them.

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

Awesome, I was just looking into this.

stores-update can be triggered at any time by e.g. cookie observers.

I think that the best approach would be to set batchTimer to only emit stores-update after window-ready in storage.js::update().

Not sure how we should handle that though.

Can you log a bug for the indexeddb issue... at least this bug fixes the original issue.

Comment 17

[Alexandre Poirot [:ochameau]]

https://hg.mozilla.org/integration/fx-team/rev/c3b3d8650c05dc992c3a6ecb853094ba590044f5
Bug 1245615 - Just clear hostVsStores map as some async code may still use them. r=mratcliffe

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c3b3d8650c05