845982 - Properly close() PCompositor and PImageBridge

Status: RESOLVED WORKSFORME

(Core :: Graphics: Layers, defect)

Description

[Benoit Girard (:BenWa)]

Attachment: patch

I was trying to find why my Shmem leak patch was not working. Turns out these protocols are not closed properly.

Should I also look into adding an ipdl assert to make sure we don't forget again? In the destructor for AsyncChannel we don't check that the channel is close. Perhaps we should.

Comment 1

[Benoit Girard (:BenWa)]

https://tbpl.mozilla.org/?tree=Try&rev=0c5970a8ebaf

Comment 2

[Benoit Girard (:BenWa)]

This causes a race, atleast on the GFX branch, because closing the channel happens Async and we release the CompositorParent in parallel.

Comment 3

[Nicolas Silva [:nical]]

BenWa, did a fix for this bug make it into the gfx branch / mozilla-central ?

Comment 4

[Joe Drew (not getting mail)]

At least some of this never made it; bug 899325 covers calling Close() on the channel at shutdown time.

Comment 6

[Joe Drew (not getting mail)]

Attachment: unbitrotted, remove commented-out code in nsBaseWidget

This is Benoit's patch, unbitrotted and with the effective changes of bug 899325 (removing the comment) rolled in.

Try: https://tbpl.mozilla.org/?tree=Try&rev=95f18d04ee8c

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 783223 [details] [diff] [review]
unbitrotted, remove commented-out code in nsBaseWidget

Review of attachment 783223 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with this change:

::: gfx/layers/ipc/ImageBridgeChild.cpp
@@ +133,5 @@
>    if (sImageBridgeChildSingleton) {
>  
>      sImageBridgeChildSingleton->SendStop();
>    }
> +  sImageBridgeChildSingleton->Close();

Shouldn't this be in the if block?

Comment 8

[Chris Peterson [:cpeterson]]

Mass tracking-e10s flag change. Filter bugmail on "2be0fcce-e36a-4e2c-aa80-0e3d33eb5406".

Comment 9

[Brad Lassey [:blassey] (use needinfo?)]

Andrew, Benoit is on leave now, do you want to pick this up?

Comment 10

[Andrew McCreight [:mccr8]]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #9)
> Andrew, Benoit is on leave now, do you want to pick this up?

I might be able to get to this at some point, but not this quarter.  I have a few other e10s leak related projects I'm working on.

Comment 11

[Benoit Girard (:BenWa)]

nical worked heavily on the shutdown. I think all this code has since changed.