Bug 1455637: Use RefPtr rather than nsCOMPtr to avoid data race. r=mccr8
authorKris Maglione <maglione.k@gmail.com>
Tue, 09 Oct 2018 13:22:28 -0700
changeset 498851 983a5ee19bb3844c4d7f00ba55bb958b01e7341c
parent 498850 8f0aa41876197fc701ddadb73eee8edaa753af19
child 498852 3854b434407792df13fad738667b042e2ebb0449
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1455637
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1455637: Use RefPtr rather than nsCOMPtr to avoid data race. r=mccr8 getter_AddRefs for nsCOMPtr does an AssertNoQueryNeeded check when its temporary is destroyed. For the mReaderThread, this happens at a time when control of the member variable has been handed over to the reader thread, which causes a data race in the query needed check when the reader thread shuts itself down and clears the member. Switching to RefPtr solves this problem by removing the unnecessary check. Differential Revision: https://phabricator.services.mozilla.com/D8135
js/xpconnect/loader/URLPreloader.h
--- a/js/xpconnect/loader/URLPreloader.h
+++ b/js/xpconnect/loader/URLPreloader.h
@@ -314,17 +314,20 @@ private:
     // The prefix URLs for files in the GRE and App omni jar archives.
     nsCString mGREPrefix;
     nsCString mAppPrefix;
 
     nsCOMPtr<nsIResProtocolHandler> mResProto;
     nsCOMPtr<nsIChromeRegistry> mChromeReg;
     nsCOMPtr<nsIFile> mProfD;
 
-    nsCOMPtr<nsIThread> mReaderThread;
+    // Note: We use a RefPtr rather than an nsCOMPtr here because the
+    // AssertNoQueryNeeded checks done by getter_AddRefs happen at a time that
+    // violate data access invariants.
+    RefPtr<nsIThread> mReaderThread;
 
     // A map of URL entries which have were either read this session, or read
     // from the last session's cache file.
     HashType mCachedURLs;
 
     Monitor mMonitor{"[URLPreloader::mMutex]"};
 };