Add comment explaining GetInProcessParentDocument's Fission behavior in WorkerPrivate::GetLoadInfo
Categories
(Core :: DOM: Workers, task, P3)
Tracking
()
Fission Milestone | Future |
People
(Reporter: kmag, Assigned: asuth)
References
(Blocks 1 open bug)
Details
It's meant to walk the parent document chain until it finds a document without a sandboxed origin, but it stops as soon as it encounters one which is OOP.
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Comment 1•4 years ago
|
||
Not sure the existing behavior is possible with fission. asuth, can you please look into what is it trying to do here and if it will need to be fixed for Fission?
Assignee | ||
Comment 2•4 years ago
|
||
The current use appears to be fine. I'm going to provide a patch to add a comment that clarifies that the logic is sound under Fission and explains why. If there is an alternate method signature that should be used that's intended to be "I've considered the fission implications", please let me know and I'll switch the code to use that too, but I didn't see one (for Document, I believe there are some for other types).
Restating:
- Per Process Per Domain Worker Limit:
- Workers has a per-process mechanism that limits the maximum number of actively running workers that can be spawned by a domain (not origin! eTLD+1). Currently the limit is 512 and pref-overrideable.
- This is different than the historical (per-process) per-domain limit of 10 active workers which was removed in bug 1280241. (Any number of Workers could be queued up, however, which created potential content-space deadlocks if they were waiting for a Worker to do something.)
- However, bug 1286895 which introduced the 512 limit it was implemented by just re-introducing that mechanism which was removed in bug 1280241 with its much higher limit.
- The intent of this limit was to avoid fork bombs (OS-level denial of service attack) and avoid filling the address space with stacks (a mechanism to make life easier for in-process attacks)
- Sandbox-piercing:
- The logic that calls GetInProcessPazrentDocument in WorkerPrivate::GetLoadInfo is attempting to locate the domain that created the sandboxed iframe and therefore is the responsible party that the worker should be billed against.
- Per question raised at https://matrix.to/#/!DAzdivXKbExPfeQEjo:mozilla.org/$XMqPM8w2VQJC_Ry3SJsYmaCzNDlZVKjMBnWTiiRT_Yw?via=mozilla.org&via=matrix.org&via=t2bot.io and subsequent discussion, although the null principals used by sandboxed iframes could in theory end up in any number of places, it's likely that the intentional policy will be for sandboxed iframes to remain in the same process as the creator of the sandboxed iframe.
- This means that the logic should continue to work for its current purposes.
- Given that our goal is to bound the number of worker threads that can be created in a given process and we should be able to properly attribute sandboxed iframes, this means our per-process accounting will be accurate. And even more accurate at a profile-wide level under fission than it was without fission!
- The question is whether there is a better alternate mechanism to get this information.
- I don't think there is at this time. The logic that creates the null principal inherits the origin attributes of the parent, but there's no linkage between that null principal and its parent and the ClientSource and I don't think there's any other comparable mechanism that would track this relationship. (It might make sense for ClientSource to expose some of this hierarchy for about:performance's benefit though? Or maybe its data gathering should already be aware of the hierarchy?)
Comment 3•4 years ago
|
||
The patch with the comment is not needed for M6b.
Comment 4•4 years ago
|
||
(In reply to Andrew Sutherland [:asuth] (he/him) from comment #2)
The current use appears to be fine. I'm going to provide a patch to add a comment that clarifies that the logic is sound under Fission and explains why.
In that case, this bug doesn't need to block shipping Fission MVP.
Updated•3 years ago
|
Comment hidden (spam) |
Description
•