Closed
Bug 1262898
Opened 8 years ago
Closed 8 years ago
Shut the the gfx ipdl protocols down properly for child processes on Windows
Categories
(Core :: Graphics: Layers, defect, P1)
Core
Graphics: Layers
Tracking
()
RESOLVED
FIXED
mozilla48
People
(Reporter: nical, Assigned: bas.schouten)
References
(Blocks 1 open bug)
Details
(Whiteboard: [gfx-noted])
Attachments
(1 file)
In bug 1215265 we improved the shutdown of gfx protocols in child processes, but it landed disabled on Windows. It can be enabled using the pref "layers.child-process-shutdown". On my last try push something was leaking a CompositorThreadHolder which causes the shutdown of the parent process to timeout. Let's figure the missing pieces on Windows and remove the pref.
Updated•8 years ago
|
Whiteboard: [gfx-noted]
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → bas
Blocks: 1244883
Assignee | ||
Comment 1•8 years ago
|
||
So the problem here is that ContentParent::ActorDestroy will post deletion of the GeckoChildProcessHost. However ImageBridgeParent or CompositorBridgeParent may still need to shutdown. Since GeckoChildProcessHost deletion will block the IO thread until the child process has died those shutdown messages will never be able to reach the parent. The GeckoChildProcessHost needs to stay alive until both the ImageBridgeParent and CompositorBridgeParent ActorDestroy functions have been called.
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/48011/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/48011/
Attachment #8743752 -
Flags: review?(nical.bugzilla)
Attachment #8743752 -
Flags: review?(jmathies)
Reporter | ||
Comment 3•8 years ago
|
||
Comment on attachment 8743752 [details] MozReview Request: Bug 1262898: Keep the GeckoChildProcessHost alive for the lifetime of the CompositorBridge and ImageBridge parent actors. r=jimm r=nical https://reviewboard.mozilla.org/r/48011/#review44805
Attachment #8743752 -
Flags: review?(nical.bugzilla) → review+
Comment 4•8 years ago
|
||
Comment on attachment 8743752 [details] MozReview Request: Bug 1262898: Keep the GeckoChildProcessHost alive for the lifetime of the CompositorBridge and ImageBridge parent actors. r=jimm r=nical https://reviewboard.mozilla.org/r/48011/#review45389 Looks ok, I was looking for ways this might break all the odd shutdown patsh we have. AFAICt it just delays the destruction of the GeckoChildProcessHost object, and that seems safe. Didn't see any issues running this in the debugger either. ::: gfx/layers/ipc/CompositorBridgeParent.cpp:2263 (Diff revision 1) > CrossProcessCompositorBridgeParent::ActorDestroy(ActorDestroyReason aWhy) > { > RefPtr<CompositorLRU> lru = CompositorLRU::GetSingleton(); > lru->Remove(this); > + > + if (mSubprocess) { Lets add a comment here explaining what this does - it sets this gecko child process host up for async deletion once additiona actors (mostly gfx) clear their associations.
Attachment #8743752 -
Flags: review?(jmathies) → review+
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4c3b5f9bd44a
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 7•8 years ago
|
||
Hey ryan, this is supposed to help with a bunch of e10s related test failures, some of which are in the blocking list.
Flags: needinfo?(ryanvm)
Comment 8•8 years ago
|
||
Yeah, it's running on Ash right now and looking very promising! The Windows M(5) permaleaks are gone :)
Flags: needinfo?(ryanvm)
Updated•8 years ago
|
tracking-e10s:
--- → +
Priority: -- → P1
You need to log in
before you can comment on or make changes to this bug.
Description
•