1117585 - Disk consumption observers could get released on the cache thread

Status: RESOLVED FIXED

Description

[[e-mail]]

Created attachment 8543686 [details]
Stack backtrace

mozilla::net::CacheIndex::ChangeState clears its list of disk consumption observers. However these observers contain weak references which aren't safe to clear on the cache thread. I'm not sure how I triggered the problem however I'd restarted after turning off e10s in options and the crash happened shortly after I closed the options tab. Stack is from an opt with symbols build so some symbols may have been merged.

Comment 1

[Michal Novotny (:michal)]

Created attachment 8544298 [details] [diff] [review]
fix

https://treeherder.mozilla.org/#/jobs?repo=try&revision=99b27c47a323

Comment 2

[[e-mail]]

Comment on attachment 8544298 [details] [diff] [review]
fix

>+        nsIWeakReference *obs;
>+        mObserver.forget(&obs);
>+
>+        nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
>+        if (mainThread) {
>+          NS_ProxyRelease(mainThread, obs);
>+        } else {
>+          NS_WARNING("Cannot get main thread, leaking weak reference to "
>+                     "CacheStorageConsumptionObserver.");
>+        }
It's a shame that NS_ProxyRelease crashes (by attempting to release the weak reference on the wrong thread) if you pass in a null thread otherwise you could just replace this with NS_ProxyRelease(mainThread, mObserver);

Comment 3

[Michal Novotny (:michal)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8068a6826e68

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8068a6826e68