Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Description • 10 years ago

I've been trying to debug bug 1120898 and I think I found a problem in how the cross process compositors are managed. The test in that bug opens and closes windows very quickly. Here's what I see happening:

1. Window is closed. The widget is torn down along with the CompositorChild and CompositorParent for that window.
2. The child process sends up a ForceComposite message.
3. The parent receives the ForceComposite message and it's routed to CrossProcessCompositorParent::ForceComposite. That code finds the CompositorParent that was deleted in step 1 and then tries to use it.

I don't see anything that is designed to prevent this from happening. I think the only reason my patch in bug 567058 triggered this is that it made it possible to create windows a little faster in e10s.

Marking s-s in case this also affects b2g.

	Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Comment 1 • 10 years ago

This fixes the immediate problem. However, I'm a little worried about the other places where we do MOZ_ASSERT(state->mParent). I left them alone, but we might want to do null checks there instead.

	Milan Sreckovic [:milan] (needinfo for best results)
	Comment 2 • 10 years ago

Wonder if this is the same root cause as bug 1120331 and bug 1120485, though I appreciate the stacks are not necessarily the same...

	Milan Sreckovic [:milan] (needinfo for best results)
	Comment 3 • 10 years ago

Comment on attachment 8549210 [details] [diff] [review]
compositor-fix

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

Did you attach the correct patch?

	Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Comment 4 • 10 years ago

No I did not.

	Nicolas Silva [:nical]
	Comment 5 • 10 years ago

Comment on attachment 8549214 [details] [diff] [review]
compositor-fix v2

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

Not sure about the other assertions you mentionned. Ideally we'd not have the possibility to shut a compositor down and still try to send updates to it (rather than null checking when it happens).

	Nicolas Silva [:nical]
	Comment 6 • 10 years ago

(In reply to Milan Sreckovic [:milan] from comment #2)
> Wonder if this is the same root cause as bug 1120331 and bug 1120485, though
> I appreciate the stacks are not necessarily the same...

I think that they are different things.

	Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Comment 7 • 10 years ago

On further thought, b2g doesn't have multiple OS windows, so I don't think this is security sensitive.

	Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Comment 8 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/dff2539946b2

	Ben Turner (not reading bugmail, use the needinfo flag!)
	Comment 9 • 10 years ago

Ooh, let's backport this as far as possible!

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 10 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/dff2539946b2