Closed Bug 877534 Opened 7 years ago Closed 7 years ago

Leaking AsyncChannel, CompositorParent, CompositorChild etc on mac with OMTC

Categories

(Core :: Graphics: Layers, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

References

Details

Attachments

(1 file)

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.
Flags: needinfo?(bent.mozilla)
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.
Flags: needinfo?(bent.mozilla)
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+
https://hg.mozilla.org/mozilla-central/rev/18a68adb330f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Depends on: 884334
You need to log in before you can comment on or make changes to this bug.