Closed Bug 1053275 Opened 10 years ago Closed 9 years ago

ServiceWorkers must not be blocked from running by 20 worker thread limitation

Categories

(Core :: DOM: Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: bkelly, Assigned: nsm)

References

Details

Attachments

(1 file)

Currently we queue up workers if there are already 20 outstanding workers for a domain.  ServiceWorkers need to be exempted from this limitation since navigation can break if they are suspended.  Also, if its that easy to prevent a ServiceWorker from running, then they will not be reliable enough to use for their intended purpose in practice.
Punting to v2 since 20 workers on a website seems unlikely to happen at this point on the web considering use of workers is fairly low and serviceworkers is non-existent.
Blocks: ServiceWorkers-B2G
No longer blocks: ServiceWorkers
(In reply to Nikhil Marathe [:nsm] (needinfo? please) from comment #1)
> Punting to v2 since 20 workers on a website seems unlikely to happen at this
> point on the web considering use of workers is fairly low and serviceworkers
> is non-existent.

Workers are the new threads: bug 1131757.
This is affecting mochitests; one option brought up is to have a pref that removes the limit.
I believe this is an issue for mochitests because each test ends up registering its own scope.  Real world apps will only experience the same thing if they register a ton of scopes that get triggered at the same time.

Exempting mochitests from the limit would be a nice short term solution.  Maybe we can get telemetry to see how many scopes real world sites are registering in the wild.
My plan is:
1) Add a testing only pref to exempt the limit for now - dom.serviceWorkers.exemptFromPerDomainMax
2) Add a telemetry probe and console warnings to RuntimeService if ServiceWorkers hit limits - Filed bug 1139513. Based on stats we can either bump up the per domain limit, have a separate limit exclusively for SWs or make them unbounded.
Adds the pref and enables it in all tests.
Attachment #8572779 - Flags: review?(bent.mozilla)
Assignee: nobody → nsm.nikhil
Status: NEW → ASSIGNED
Can we get someone else to review this?  At this point, any new SW test landing means that some other test starts to fail too often, hindering landing new stuff.
Flags: needinfo?(overholt)
Comment on attachment 8572779 [details] [diff] [review]
Exempt ServiceWorkers from per domain thread limits

Jonas told me he could take this review while Ben's on PTO.
Flags: needinfo?(overholt)
Attachment #8572779 - Flags: review?(bent.mozilla) → review?(jonas)
Comment on attachment 8572779 [details] [diff] [review]
Exempt ServiceWorkers from per domain thread limits

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

r=me assuming that you're sure you're always on main thread.

::: dom/workers/RuntimeService.cpp
@@ +1372,5 @@
>    }
>  
> +  bool exemptFromPerDomainMax = false;
> +  if (aWorkerPrivate->IsServiceWorker()) {
> +    exemptFromPerDomainMax = Preferences::GetBool("dom.serviceWorkers.exemptFromPerDomainMax",

Are you sure this will only be on the main thread? Is that because SWs never have a parent thread? At least add an assertion that you're on the main thread.
Attachment #8572779 - Flags: review?(jonas) → review+
https://hg.mozilla.org/mozilla-central/rev/36c32d9b014e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: