Intermittent test_serviceworker_lifetime.html | Service worker was terminated, | Test timed out, etc.

RESOLVED FIXED in Firefox 49

Status

()

RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: philor, Assigned: bkelly)

Tracking

({intermittent-failure})

unspecified
mozilla51
intermittent-failure
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox48 unaffected, firefox49 fixed, firefox50 fixed, firefox51 fixed)

Details

Attachments

(1 attachment)

Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
status-firefox48: --- → unaffected
status-firefox49: --- → affected
status-firefox50: --- → affected
Any ideas, Kit?
Flags: needinfo?(kcambridge)
Comment hidden (Intermittent Failures Robot)
Not yet. :-( Keeping ni? to follow up, but I won't have cycles to look into this for a while.
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Any ideas Ben? This is really frequent on Beta too.
Flags: needinfo?(bkelly)
Comment hidden (Intermittent Failures Robot)
I'll look on Monday.  I'm on PTO tomorrow.
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
(Assignee)

Updated

2 years ago
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Flags: needinfo?(bkelly)
Created attachment 8786091 [details] [diff] [review]
Avoid racing with initial shutdown in test_service_worker_lifetime.html. r=kitcambridge

https://treeherder.mozilla.org/#/jobs?repo=try&revision=26ccdf91a0b3
Comment on attachment 8786091 [details] [diff] [review]
Avoid racing with initial shutdown in test_service_worker_lifetime.html. r=kitcambridge

Try is green with ~30 pushes on linux64 pgo.  So I think this is good to go.

Kit, I believe the issue here is that the initial service worker instance spawned during the createIframe() call was sometimes shutting down right as the push event was being processed.  Normally the timing works our ok, but in pgo we start losing the race.

This patch avoids the problem by waiting to proceed until this initial service worker thread has positively been stopped.
Flags: needinfo?(kcambridge)
Attachment #8786091 - Flags: review?(kcambridge)
Comment on attachment 8786091 [details] [diff] [review]
Avoid racing with initial shutdown in test_service_worker_lifetime.html. r=kitcambridge

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

Thanks, Ben!
Attachment #8786091 - Flags: review?(kcambridge) → review+
Ben explained this to me a bit more on IRC. The explanation was very helpful, so capturing it here for posterity...

09:35 <bkelly> whats up?
09:36 <kitcambridge> bkelly: just trying to understand what's going on with that test. :-) so, createIframe spins up the service worker...
09:36 <kitcambridge> but it has nothing to do, so it shuts down
09:36 <bkelly> kitcambridge: right, because idle timer is set to zero
09:36 <kitcambridge> and that races with the setShutdownObserver(false)?
09:36 <bkelly> kitcambridge: but it still takes some time to shutdown
09:37 <bkelly> kitcambridge: and in non-pgo sometimes it shuts down after setShutdownObserver(false), but before we can fire the push event to trigger the waitUntil()
09:37 <bkelly> sorry in pgo
09:37 <kitcambridge> and in that case, we'll observe the earlier shutdown instead of the one caused by the push event
09:38 <bkelly> right
09:38 <kitcambridge> so your patch fixes that so it waits for the first shutdown
09:38 <kitcambridge> and after that, we attach another observer and fire the event
09:39 <bkelly> and that event uses a waitUntil() to keep it alive
09:39 <bkelly> so we have no shutdown race

Comment 31

2 years ago
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/31b100c982b5
Avoid racing with initial shutdown in test_service_worker_lifetime.html. r=kitcambridge
Let me see if this works in FF49.
Comment on attachment 8786091 [details] [diff] [review]
Avoid racing with initial shutdown in test_service_worker_lifetime.html. r=kitcambridge

The patch applies cleanly to beta and runs locally.

Approval Request Comment
[Feature/regressing bug #]: Service workers
[User impact if declined]: Test intermittents annoying RyanVM on beta/aurora.
[Describe test coverage new/current, TreeHerder]: The patch modifies an existing mochitest.
[Risks and why]: Minimal.  Only modified a test.
[String/UUID change made/needed]: None
Attachment #8786091 - Flags: approval-mozilla-beta?
Attachment #8786091 - Flags: approval-mozilla-aurora?

Comment 34

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/31b100c982b5
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment hidden (Intermittent Failures Robot)
Attachment #8786091 - Flags: approval-mozilla-beta?
Attachment #8786091 - Flags: approval-mozilla-aurora?

Comment 36

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/145d2272fe84
status-firefox50: affected → fixed
Flags: in-testsuite+

Comment 37

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/faf186e4d05c
status-firefox49: affected → fixed
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
You need to log in before you can comment on or make changes to this bug.