Closed Bug 877534 Opened 7 years ago Closed 7 years ago
Channel, Compositor Parent, Compositor Child etc on mac with OMTC
I think this is a regression, or a remaining unfixed issue from bug 848949. Seems to be fairly intermittent, but if you enable the OMTC pref and run all OSX mochitests you should hit it at least once. Log file: https://tbpl.mozilla.org/php/getParsedLog.php?id=23567523&tree=Try&full=1#error0
This leak happens because the task we schedule to release the CompositorParent/Child never happens. We schedule it here: http://mxr.mozilla.org/mozilla-central/source/widget/xpwidgets/nsBaseWidget.cpp#148 This happens pretty late in the shutdown sequence, after the 'NOTE: parent process received `Goodbye', closing down' message is printed. Anyone have ideas about why this message might not be processed, and how we can fix it?
I haven't been able to reproduce this locally yet. It appears that we don't call MessageLoop::Quit on the main thread's message loop though. I put a breakpoint on it, and only get callers from ThreadQuitTask on other threads.
The main thread event loop is really complicated and it's been ages since I've looked at it. We'll need to debug whether MessageLoop::Quit is necessary, and if it is why it isn't being called.
It looks like we do attempt to clear the event queue, but sometimes we end up with an nsIWidget reference that doesn't get released until the cycle collector shuts down. Unfortunately this is too late in the shutdown sequence, and any events that it adds never get processed. This patch just adds a listener for the xpcom shutdown event and makes sure that we destroy the compositor at that point.
Attachment #763206 - Flags: review?(roc)
Comment on attachment 763206 [details] [diff] [review] Add a widget shutdown observer Review of attachment 763206 [details] [diff] [review]: ----------------------------------------------------------------- r+ with those fixed. ::: widget/xpwidgets/nsBaseWidget.h @@ +394,5 @@ > nsSizeMode mSizeMode; > nsPopupLevel mPopupLevel; > nsPopupType mPopupType; > SizeConstraints mSizeConstraints; > + nsCOMPtr<WidgetShutdownObserver> mShutdownObserver; Move this up near other pointer-like fields. @@ +395,5 @@ > nsPopupLevel mPopupLevel; > nsPopupType mPopupType; > SizeConstraints mSizeConstraints; > + nsCOMPtr<WidgetShutdownObserver> mShutdownObserver; > + bool mIsShutdown; I don't think we need this bool. We can just check whether mShutdownObserver is null.
Attachment #763206 - Flags: review?(roc) → review+
Assignee: bgirard → matt.woodrow
Push backed out for Android mochitest-3 crashes: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=8ce5f3f4937d&jobname=android.*moch.*3 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/347f1820d874 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/291427f3bb3a remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/d857c7f2fc2f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.