Bug 1727405 - Add a diagnostic assert to check that ClientManager::GetOrCreateForCurrentThread is not returning a stale ClientManager instance that is already shutdown. r=asuth

While investigating an unexpected test failure triggered by a mochitest that was testing the identity WebExtensions API
(D121683 from Bug 1723852) I did notice that the actual underlying issue was triggered by a leak
(in particular the Extension API class in the initial draft of that D121683 patch was missing a RefPtr
in the NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE macro used for that class) but the issue was presenting
itself as an empty scriptURL in the ClientSource for the test case that was executed after the one that
triggered tha leak, and far enough from where it was actually triggered.

To make it easier to spot the issue nearer to the actual underlying issue, I think that it would be
reasonable to add a diagnostic assertion to ClientManager::GetOrCreateForCurrentThread that would
be triggered earlier if a leak was keeping the ClientManager instance alive in the idle DOM Worker
Thread.

Differential Revision: https://phabricator.services.mozilla.com/D123530
This commit is contained in:
Luca Greco 2021-08-31 14:18:35 +00:00
parent ac7da6c730
commit 0062a37d58

View File

@ -217,6 +217,12 @@ already_AddRefed<ClientManager> ClientManager::GetOrCreateForCurrentThread() {
}
MOZ_DIAGNOSTIC_ASSERT(cm);
// Check that the ClientManager instance associated to the current thread has
// not been kept alive when it was expected to have been already deallocated
// (e.g. due to a leak ClientManager's mShutdown can have ben set to true from
// its RevokeActor method but never fully deallocated and unset from the
// thread locals).
MOZ_DIAGNOSTIC_ASSERT(!cm->IsShutdown());
return cm.forget();
}