Closed Bug 1439383 Opened 2 years ago Closed 2 years ago

the LoadRoots background thread hangs around indefinitely

Categories

(Core :: Security: PSM, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: froydnj, Assigned: keeler)

References

Details

(Whiteboard: [MemShrink:P3][psm-assigned])

Attachments

(1 file)

LoadLoadableRootsTask::Dispatch creates a thread for itself to run on, but never shuts the thread down.  So we have this single-use thread hanging out in the parent process, consuming resources (mostly the memory for its stack) for no reason.

We should shut down the thread after LoadLoadableRootsTask has finished its work, or find a separate, non-transient thread to do this work on.
Whiteboard: [MemShrink] → [MemShrink:P3]
Priority: -- → P2
Whiteboard: [MemShrink:P3] → [MemShrink:P3][psm-backlog]
Assignee: nobody → dkeeler
Priority: P2 → P1
Whiteboard: [MemShrink:P3][psm-backlog] → [MemShrink:P3][psm-assigned]
Comment on attachment 8960276 [details]
bug 1439383 - clean up the load loadable roots thread when we're done with it

https://reviewboard.mozilla.org/r/229044/#review234816

Thank you!
Attachment #8960276 - Flags: review?(nfroyd) → review+
Comment on attachment 8960276 [details]
bug 1439383 - clean up the load loadable roots thread when we're done with it

https://reviewboard.mozilla.org/r/229044/#review234946

LGTM.
Attachment #8960276 - Flags: review?(jjones) → review+
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5be07e86738e
clean up the load loadable roots thread when we're done with it r=froydnj,jcj
https://hg.mozilla.org/mozilla-central/rev/5be07e86738e
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Depends on: 1401883
Backout by csabou@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/c23c7481957f
Backed out changeset 5be07e86738e for causing leaks (bug 1401883). a=backout
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I tried some things, but I have no idea why shutting down a thread would cause leaks. :froydnj any ideas?
Flags: needinfo?(dkeeler) → needinfo?(nfroyd)
(In reply to David Keeler [:keeler] (use needinfo) from comment #9)
> I tried some things, but I have no idea why shutting down a thread would
> cause leaks. :froydnj any ideas?

I have no idea either, that seems bizarre.  The one thing that comes to mind is that with this patch, you're now sending an extra event through the event loop, and that does...something.  It changes some sort of timing window just enough that the conditions for the leak happen more consistently?

Unfortunately, the leaks only seem to happen on Windows, so we don't have rr available...maybe MS's time travel debugging could be used to capture this?
Flags: needinfo?(nfroyd)
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ffd4b055fb8b
clean up the load loadable roots thread when we're done with it r=froydnj,jcj
https://hg.mozilla.org/mozilla-central/rev/ffd4b055fb8b
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.