1273795 - Intermittent browser_net_service-worker-status.js | FATAL ERROR: AsyncShutdown timeout in ShutdownLeaks: Wait for cleanup to be finished before checking for leaks Conditions: [{"name":"DevTools: Wait until toolbox is destroyed","state":"(none)","filename"

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P1)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://treeherder.mozilla.org/logviewer.html#?job_id=28104891&repo=mozilla-inbound

Comment 1

[Intermittent Failures Robot]

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

Repository breakdown:
* mozilla-inbound: 6
* mozilla-aurora: 1

Platform breakdown:
* osx-10-10: 6
* windows7-32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1273795&startday=2016-05-16&endday=2016-05-22&tree=all

Comment 2

[Intermittent Failures Robot]

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

Repository breakdown:
* mozilla-inbound: 6
* fx-team: 2
* mozilla-aurora: 1

Platform breakdown:
* osx-10-10: 6
* windows7-32: 2
* linux64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1273795&startday=2016-05-23&endday=2016-05-29&tree=all

Comment 3

[Intermittent Failures Robot]

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

Repository breakdown:
* mozilla-inbound: 4
* try: 1
* mozilla-central: 1
* mozilla-aurora: 1

Platform breakdown:
* osx-10-10: 6
* linux64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1273795&startday=2016-05-30&endday=2016-06-05&tree=all

Comment 4

[Intermittent Failures Robot]

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

Repository breakdown:
* mozilla-beta: 2
* mozilla-central: 1
* fx-team: 1
* ash: 1

Platform breakdown:
* osx-10-10: 4
* windows7-32: 1

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

Comment 5

[Jarda Snajdr [:jsnajdr]]

I suspect this is caused by the assertions and errors described in bug 1293980.

Comment 6

[Jarda Snajdr [:jsnajdr]]

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

Comment 7

[Jarda Snajdr [:jsnajdr]]

Attachment: Bug 1273795 - browser_net_service-worker-status.js: use ContentTask instead of CPOW

Review commit: https://reviewboard.mozilla.org/r/71182/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71182/

Comment 8

[Jarda Snajdr [:jsnajdr]]

Another possible why this test can fail, in addition to the securityInfo issue in bug 1293980, is that it resolves promises with a CPOW when registering and unregistering the service worker. See bug 1233497. I'm converting all CPOW calls with a ContentTask.

The test will start permafailing when you remove the "skipPermitUnload" parameter to removeTab at [1]. I'm going to do exactly this in another cleanup, where I start including a shared-head.js in netmonitor tests, and unify several duplicate implementations of removeTab into one.

[1] http://searchfox.org/mozilla-central/source/devtools/client/netmonitor/test/head.js#117

Comment 9

[Alexandre Poirot [:ochameau]]

Comment on attachment 8780482 [details]
Bug 1273795 - browser_net_service-worker-status.js: use ContentTask instead of CPOW

https://reviewboard.mozilla.org/r/71182/#review69122

Thanks for this cleanup.

::: devtools/client/netmonitor/test/browser_net_service-worker-status.js:47
(Diff revision 1)
>    info("Performing requests...");
> -  debuggee.performRequests();
> +  yield ContentTask.spawn(tab.linkedBrowser, {}, function* () {
> +    content.wrappedJSObject.performRequests();
> +  });
> +
>    yield waitForNetworkEvents(monitor, REQUEST_DATA.length);

Couldn't that race?
It is a typical pattern where events may be fired while we wait for yield ContentTask.spawn.

To be less prone to race, you would do:
  let onEvents = waitForNetworkEvents();
  yield ContentTask.spawn(...performRequest...); // This yield looks optional
  yield onEvents;

Comment 10

[Jarda Snajdr [:jsnajdr]]

Comment on attachment 8780482 [details]
Bug 1273795 - browser_net_service-worker-status.js: use ContentTask instead of CPOW

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/71182/diff/1-2/

Comment 11

[Jarda Snajdr [:jsnajdr]]

Comment on attachment 8780482 [details]
Bug 1273795 - browser_net_service-worker-status.js: use ContentTask instead of CPOW

https://reviewboard.mozilla.org/r/71182/#review69122

> Couldn't that race?
> It is a typical pattern where events may be fired while we wait for yield ContentTask.spawn.
> 
> To be less prone to race, you would do:
>   let onEvents = waitForNetworkEvents();
>   yield ContentTask.spawn(...performRequest...); // This yield looks optional
>   yield onEvents;

Yes, it could race, thanks for pointing it out. Posted an updated patch with:
- use add_task and remove call to finish()
- waitForNetworkEvents() without a race
- yield in ContentTask.spawn(...performRequest...) can be useful for catching errors in the task

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a94448be42e2
browser_net_service-worker-status.js: use ContentTask instead of CPOW r=ochameau

Comment 13

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

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

Comment 14

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/c6f75c8af7d0