797515 - Race condition in MOZ_ASSERT in ContentParent::~ContentParent()

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Justin Lebar (not reading bugmail)]

If !mAppManifestURL.IsEmpty(), ContentParent::~ContentParent() asserts

  MOZ_ASSERT(!gAppContentParents ||
             !gAppContentParents->Get(mAppManifestURL));

At first glance, this assert should pass if we always remove |this| from gAppContentParents before deleting this, and if there's only one process per app manifest.

But I think this is a race because the ContentParent destructor is run asynchronously.  It might be the case that after we committed to destroying this ContentParent, we created another one for the same app.

I think this is what's happening in bug 789392.  Patch in a moment.

Comment 1

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v1

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 667623 [details] [diff] [review]
Patch, v1

Nice!

Comment 3

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/99caa1f41fe1

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Guess that didn't work. Backed out for M2 orange.
https://hg.mozilla.org/integration/mozilla-inbound/rev/16ac5cbc7793

https://tbpl.mozilla.org/php/getParsedLog.php?id=15792667&tree=Mozilla-Inbound

199 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/browser-element/mochitest/test_browserElement_inproc_CloseApp.html | Should have gotten app frame close by now.
588 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/browser-element/mochitest/test_browserElement_oop_CloseApp.html | Should have gotten app frame close by now.

Comment 5

[Ed Morley [:emorley]]

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