Closed
Bug 1348547
Opened 8 years ago
Closed 8 years ago
Intermittent devtools/client/aboutdebugging/test/browser_service_workers_status.js | Service worker is currently running - Got Stopped, expected Running
Categories
(DevTools :: about:debugging, defect)
DevTools
about:debugging
Tracking
(firefox53 unaffected, firefox54 fixed, firefox55 fixed)
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox53 | --- | unaffected |
firefox54 | --- | fixed |
firefox55 | --- | fixed |
People
(Reporter: aryx, Assigned: gbrown)
References
Details
(Keywords: intermittent-failure, Whiteboard: [stockwell fixed])
Attachments
(1 file)
620 bytes,
patch
|
gkrizsanits
:
review+
|
Details | Diff | Splinter Review |
https://treeherder.mozilla.org/logviewer.html#?job_id=84722558&repo=autoland
13:30:11 INFO - Entering test bound
13:30:11 INFO - opening about:debugging
13:30:11 INFO - Adding a new tab with URL: about:debugging#workers
13:30:11 INFO - Buffered messages logged at 13:30:06
13:30:11 INFO - Tab added and finished loading
13:30:11 INFO - Adding a new tab with URL: http://example.com/browser/devtools/client/aboutdebugging/test/service-workers/delay-sw.html
13:30:11 INFO - Buffered messages logged at 13:30:07
13:30:11 INFO - Tab added and finished loading
13:30:11 INFO - Make the test page notify us when the service worker sends a message.
13:30:11 INFO - Buffered messages logged at 13:30:11
13:30:11 INFO - TEST-PASS | devtools/client/aboutdebugging/test/browser_service_workers_status.js | Found the service worker in the list -
13:30:11 INFO - Buffered messages finished
13:30:11 INFO - TEST-UNEXPECTED-FAIL | devtools/client/aboutdebugging/test/browser_service_workers_status.js | Service worker is currently running - Got Stopped, expected Running
13:30:11 INFO - Stack trace:
13:30:11 INFO - chrome://mochikit/content/browser-test.js:test_is:911
13:30:11 INFO - chrome://mochitests/content/browser/devtools/client/aboutdebugging/test/browser_service_workers_status.js:null:44
13:30:11 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:735:9
13:30:11 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:655:7
13:30:11 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:791:59
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment 3•8 years ago
|
||
another about:debugging failure from march 17th, possibly related to bug 1329836 ?
Whiteboard: [stockwell needswork]
Comment 4•8 years ago
|
||
believe this might be fixed by bug 1329836
Comment 5•8 years ago
|
||
most likely related to https://bugzilla.mozilla.org/show_bug.cgi?id=1348112 which I narrowed down for bug 1329836.
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) |
![]() |
Assignee | |
Comment 13•8 years ago
|
||
(In reply to Joel Maher ( :jmaher) from comment #5)
> most likely related to https://bugzilla.mozilla.org/show_bug.cgi?id=1348112
> which I narrowed down for bug 1329836.
I am not so sure, since bug 1329836 started much earlier (though 1329836 did increase in frequency just before 03-18).
![]() |
Assignee | |
Comment 14•8 years ago
|
||
This test was updated around 03-16 - bug 1345932. But failures did not start with those changes.
![]() |
Assignee | |
Comment 15•8 years ago
|
||
![]() |
Assignee | |
Comment 16•8 years ago
|
||
Retriggers on comment 15 now clearly indicate that this started with https://hg.mozilla.org/integration/autoland/rev/5e664d9cc14d3331264be607ae27c650352066b6 / bug 1336398, which simply increased the process count.
:mrbkap -- mostly just making you aware, but wouldn't mind if you had a look at the failure here! (It is a frequent failure, so we need someone to fix or disable the test soon).
Blocks: 1336398
Flags: needinfo?(mrbkap)
Comment 17•8 years ago
|
||
That is very surprising. Service worker debugging doesn't work in e10s-multi and these tests attempt to force a single content process [1][2]. So bug 1336398 really should have little to no effect on these tests. That suggests a bug in our process selector.
[1] http://searchfox.org/mozilla-central/rev/72fe012899a1b27d34838ab463ad1ae5b116d76b/devtools/client/aboutdebugging/test/browser_service_workers_status.js#15
[2] http://searchfox.org/mozilla-central/rev/72fe012899a1b27d34838ab463ad1ae5b116d76b/devtools/client/aboutdebugging/test/head.js#425,433
Comment hidden (Intermittent Failures Robot) |
Comment 19•8 years ago
|
||
(In reply to Blake Kaplan (:mrbkap) from comment #17)
> That is very surprising. Service worker debugging doesn't work in e10s-multi
> and these tests attempt to force a single content process [1][2]. So bug
> 1336398 really should have little to no effect on these tests. That suggests
> a bug in our process selector.
>
> [1]
> http://searchfox.org/mozilla-central/rev/
> 72fe012899a1b27d34838ab463ad1ae5b116d76b/devtools/client/aboutdebugging/test/
> browser_service_workers_status.js#15
> [2]
> http://searchfox.org/mozilla-central/rev/
> 72fe012899a1b27d34838ab463ad1ae5b116d76b/devtools/client/aboutdebugging/test/
> head.js#425,433
Maybe it's about the tables processes we keep alive... Have you tried calling Services.ppmm.releaseCachedProcesses(); if it fixes the problem?
![]() |
Assignee | |
Comment 20•8 years ago
|
||
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #19)
> Maybe it's about the tables processes we keep alive... Have you tried
> calling Services.ppmm.releaseCachedProcesses(); if it fixes the problem?
Thanks! I'm trying https://treeherder.mozilla.org/#/jobs?repo=try&revision=ebde66316c44e062561ef39beaf438bb6484299a ... mixture of positive and negative signs so far. I'll check on that again on Monday...
Flags: needinfo?(gbrown)
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
![]() |
Assignee | |
Comment 23•8 years ago
|
||
Following up on your comment 19, I found that calling releaseCachedProcesses() does indeed appear to avoid this failure, but I saw new failures in non-e10s mode, particularly leaks. See https://treeherder.mozilla.org/#/jobs?repo=try&revision=ebde66316c44e062561ef39beaf438bb6484299a.
If I call releaseCachedProcesses() only if gMultiProcessBrowser, all is well:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc19e2dd2dddfe0b913313fe58473b572b8eb7c3
This appears to also avoid the frequent intermittent leaks in aboutdebugging mochitests reported in bug 1329836! :)
Flags: needinfo?(gbrown)
Attachment #8854152 -
Flags: review?(gkrizsanits)
Comment 24•8 years ago
|
||
(In reply to Geoff Brown [:gbrown] from comment #23)
> Following up on your comment 19, I found that calling
> releaseCachedProcesses() does indeed appear to avoid this failure, but I saw
> new failures in non-e10s mode, particularly leaks.
I wonder if ContentParent::ReleaseCachedProcesses should avoid creating sBrowserContentParents if it doesn't already exist.
Flags: needinfo?(mrbkap)
Comment hidden (Intermittent Failures Robot) |
Comment 26•8 years ago
|
||
(In reply to Blake Kaplan (:mrbkap) from comment #24)
> (In reply to Geoff Brown [:gbrown] from comment #23)
> > Following up on your comment 19, I found that calling
> > releaseCachedProcesses() does indeed appear to avoid this failure, but I saw
> > new failures in non-e10s mode, particularly leaks.
>
> I wonder if ContentParent::ReleaseCachedProcesses should avoid creating
> sBrowserContentParents if it doesn't already exist.
Yeah... I have a patch for that for the in preallocated process manager work just haven't landed it yet :(
Comment 27•8 years ago
|
||
Comment on attachment 8854152 [details] [diff] [review]
call releaseCachedProcesses()
Review of attachment 8854152 [details] [diff] [review]:
-----------------------------------------------------------------
Could you please try instead of using gMultiProcessBrowser in the test file adding this changeset to ContentParent.cpp and calling Services.ppmm.releaseCachedProcesses() unconditionally in the test?
/*static*/ void
ContentParent::ReleaseCachedProcesses()
{
+ if (!GetPoolSize(NS_LITERAL_STRING(DEFAULT_REMOTE_TYPE))) {
+ return;
+ }
+
If that works please land it that way (it should), in case it does not then land your version and I'll look into it in a followup.
Attachment #8854152 -
Flags: review?(gkrizsanits) → review+
![]() |
Assignee | |
Comment 28•8 years ago
|
||
That does work - thanks!
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a6f6fd4ad02458c2a3287d0d676baabb3caab40
Assignee: nobody → gbrown
Whiteboard: [stockwell needswork] → [stockwell fixed]
Comment 29•8 years ago
|
||
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8d8379d9a545
ReleaseCachedProcesses() between aboutdebugging tests; r=gabor
Comment hidden (Intermittent Failures Robot) |
Comment 31•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Updated•8 years ago
|
status-firefox54:
--- → affected
Updated•8 years ago
|
Comment 32•8 years ago
|
||
Can we get a rebased patch for Aurora and an approval request please?
Flags: needinfo?(gbrown)
![]() |
Assignee | |
Comment 33•8 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #32)
> Can we get a rebased patch for Aurora and an approval request please?
We'll need bug 1345932 on aurora first. There are very few of these failures on aurora...worth the effort?
Comment 34•8 years ago
|
||
Only because they've been getting incorrectly starred as bug 1352586.
Comment hidden (Intermittent Failures Robot) |
Comment 37•8 years ago
|
||
bugherder uplift |
Comment hidden (Intermittent Failures Robot) |
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•