Closed Bug 1220308 Opened 4 years ago Closed 4 years ago

extra entries accumulate in about:debugging

Categories

(Core :: DOM: Service Workers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox43 --- wontfix
firefox44 --- fixed
firefox45 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: bwalker, Assigned: bkelly)

References

Details

Attachments

(2 files)

STR:
1. visit a page which registers a serviceworker
2. rapidly alternate between cmd-R and shift-cmd-R (on Mac OS X)
3. visit about:debugging and look at workers

Expected:
* exactly one service worker listed

Actual:
* many duplicate service workers registered
baku, any thoughts here?

Bill: we're hoping to transition users from about:serviceworkers to about:debugging soon so we're hoping to spend little effort on about:sw :)
Flags: needinfo?(amarchesini)
I suspect this is related to bug 1201498 comment 4.
I cannot reproduce this issue. Can you give me your serviceWorker script and the registration page?
Flags: needinfo?(amarchesini)
Flags: needinfo?(bwalker)
(In reply to Andrea Marchesini (:baku) from comment #3)
> I cannot reproduce this issue. Can you give me your serviceWorker script and
> the registration page?

I can reproduce this by repeatedly reloading and shift-reloading either of these pages:

https://mozilla.github.io/platatus
https://wfwalker.github.io/ebird-mybird

and their corresponding service workers:

https://mozilla.github.io/platatus/offline-worker.js
https://wfwalker.github.io/ebird-mybird/offline-worker.js

Seeing it now in 45.0a1 (2015-11-05) on Mac OS X 10.10.4

Reach me any time on #irc as bwalker if I can help further
Flags: needinfo?(bwalker)
I see this too.  I just spammed ctrl-r and shift-ctrl-r on a SW enabled page.
The went away pretty fast in my case.
(In reply to Ben Kelly [:bkelly] from comment #6)
> The went away pretty fast in my case.

Mine are lingering for hours, I would say. I just unregistered all the SW's, waiting to see if they disappear now.
(In reply to Bill Walker [:bwalker] [@wfwalker] from comment #7)
> Mine are lingering for hours, I would say. I just unregistered all the SW's,
> waiting to see if they disappear now.

Yes, I see this on your platatus site as well.  Maybe Cache or something else is leaking a Worker feature.  Andrea, what do you think?
Flags: needinfo?(amarchesini)
It is indeed the Cache API worker feature holding the worker alive.
Flags: needinfo?(amarchesini)
Patch coming shortly.
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
This fixes it in my local testing.  Bill, does this work for you?
Attachment #8685521 - Flags: review?(amarchesini)
Attachment #8685521 - Flags: feedback?(bwalker)
This affects Cache API in all branches, but in practice is probably only really matters where ServiceWorker is enabled.
Actually, the patch makes it harder to trigger.  But something sometimes still keeps the workers alive.  Its not a cache feature, though.  Let me see if its a fetch() feature.
I'm having difficulty tracking down where the extra worker wedges.  We should probably implement bug 1217367 to avoid spinning up so many updates simultaneously, though.
Depends on: 1217367
No longer depends on: 1217367
See Also: → 1217367
Attachment #8685521 - Flags: review?(amarchesini) → review+
Andrea, do you understand why we are spinning up a worker private or worker thread for an update that will pass the byte-for-byte comparison?  It seems we should never get to starting the worker in that case.

But the debugger suggests to me thats why we get a new worker entry on every refresh.  Its the SoftUpdate triggered from the navigation.
Flags: needinfo?(amarchesini)
(In reply to Ben Kelly [:bkelly] from comment #11)
> Created attachment 8685521 [details] [diff] [review]
> Cache API should teardown DOM objects when Worker hits Terminating status.
> r=baku
> 
> This fixes it in my local testing.  Bill, does this work for you?

Hi Ben, I grabbed the Mac binary from your try build, and indeed the extra workers now disappear after a few seconds. Looking good. thanks!
https://hg.mozilla.org/mozilla-central/rev/10e861d62dcc
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Comment on attachment 8685521 [details] [diff] [review]
Cache API should teardown DOM objects when Worker hits Terminating status. r=baku

Approval Request Comment
[Feature/regressing bug #]: Cache API when used with Service Workers.
[User impact if declined]: We are trying to ship Service Workers on 44 if possible.  This is an issue with leaking worker threads when Cache API is used in a service worker.  It could lead to failed updates and unexpected behavior for the user.
[Describe test coverage new/current, TreeHerder]: Existing mochitests and local testing.
[Risks and why]: Minimal.  It only affects Cache API on workers.  While Cache API is shipped in release, it is not in heavy use yet since its mainly associated with service workers.
[String/UUID change made/needed]: None
Attachment #8685521 - Flags: feedback?(bwalker) → approval-mozilla-aurora?
Comment on attachment 8685521 [details] [diff] [review]
Cache API should teardown DOM objects when Worker hits Terminating status. r=baku

SW is planning to ship in FF44, seems safe to uplift to Aurora44.
Attachment #8685521 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(amarchesini)
You need to log in before you can comment on or make changes to this bug.