1230030 - Intermittent e10s TEST-UNEXPECTED-TIMEOUT | /_mozilla/service-workers/service-worker/update-after-oneday.https.html | Update should be triggered after a functional event when last update time is over 24 hours - Test timed out

Status: RESOLVED FIXED

(Testing :: web-platform-tests, defect)

Description

[Phil Ringnalda (:philor)]

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

Comment 1

[Intermittent Failures Robot]

17 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 12
* fx-team: 4
* mozilla-central: 1

Platform breakdown:
* osx-10-10: 17

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1230030&startday=2015-12-22&endday=2015-12-22&tree=all

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

NI myself to investigate this after I get back from PTO in January.

Comment 3

[Intermittent Failures Robot]

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

Repository breakdown:
* mozilla-inbound: 45
* fx-team: 12
* mozilla-central: 7

Platform breakdown:
* osx-10-10: 64

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1230030&startday=2015-12-21&endday=2015-12-27&tree=all

Comment 4

[Intermittent Failures Robot]

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

Repository breakdown:
* mozilla-inbound: 19
* fx-team: 7
* mozilla-central: 3
* b2g-inbound: 1

Platform breakdown:
* osx-10-10: 28
* linux64: 1
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1230030&startday=2015-12-28&endday=2016-01-03&tree=all

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

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

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Note sure if its the problem here, but it seems we are attempting to schedule two timers for each FetchEvent.  The first for the functional event firing and the second after the document load completes.  They should be coalesced, but maybe if the server is running particularly slow we can get an extra update in there.

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

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

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

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

It looks like we're not getting a fetch event at all for the sub-resource load.  In this case its an image.  I wonder if image cache is screwing things up somehow.

Here is a try that does cache busting on the image:

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

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

Cache busting did not help.

More debugging shows, however, that ServiceWorkerPrivate::TerminateWorker() can be called even if we have a KeepAliveToken supposedly holding the worker alive.  It seems the FetchEventRunnable is getting created and the work is being killed before it can be executed on the worker.

Lets try aborting TerminateWorker if mTokenCount is > 0:

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

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

That was a bad patch in comment 9.  We need to be able to force terminate a worker even if it has an mTokenCount.

In this case, though, its happening because the ServiceWorkerInfo object is being destroyed.  This is quite odd because it should be held alive as the reg.active worker.  More try debug...

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

So what is happening is this:

1) First navigation update triggers a PropagateRegistration()
2) This async calls back to the child process with LoadRegistration()
3) LoadRegistration() checks to see if it needs to start a new active worker or not
4) Logic error in (3) causes us to replace our current active worker with a new active worker using the same script spec.
5) Replacing the current active worker kills the WorkerPrivate and its thread.
6) It just happens to do this between creating the FetchEventRunnable and executing it on the worker thread.

Here is a try build that attempts to fix the logic error LoadRegistration():

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

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Don't replace active worker unnecessarly after saving registration in e10s mode. r=ehsan

Comment 13

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8705742 [details] [diff] [review]
Don't replace active worker unnecessarly after saving registration in e10s mode. r=ehsan

20 green try triggers.  I think this fixes the problem.

Comment 14

[(no longer active)]

Interesting.  I wonder if bug 1232555 is somehow related to this...

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b82dc4ab197b

Comment 16

[Phil Ringnalda (:philor)]

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

Comment 17

[Intermittent Failures Robot]

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

Repository breakdown:
* mozilla-inbound: 25
* fx-team: 14
* mozilla-central: 6

Platform breakdown:
* osx-10-10: 43
* linux64: 1
* linux32: 1

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