From 7bd0ac22b6789bbfe7cc39fe068b3c1d9be1e218 Mon Sep 17 00:00:00 2001 From: Aaron Klotz Date: Tue, 8 Apr 2014 22:57:52 -0600 Subject: [PATCH] Bug 913653: Remove lock from IOInterposer and add IOInterposer thread registration; r=froydnj --- dom/src/storage/DOMStorageDBThread.cpp | 3 + ipc/chromium/src/base/thread.cc | 3 + netwerk/cache2/CacheIOThread.cpp | 3 + startupcache/StartupCache.cpp | 3 + xpcom/build/IOInterposer.cpp | 590 +++++++++++++++---------- xpcom/build/IOInterposer.h | 20 +- xpcom/threads/nsThread.cpp | 5 + 7 files changed, 398 insertions(+), 229 deletions(-) diff --git a/dom/src/storage/DOMStorageDBThread.cpp b/dom/src/storage/DOMStorageDBThread.cpp index cbc9797c5bf..8ddaf41f9c0 100644 --- a/dom/src/storage/DOMStorageDBThread.cpp +++ b/dom/src/storage/DOMStorageDBThread.cpp @@ -20,6 +20,7 @@ #include "mozIStorageFunction.h" #include "nsIObserverService.h" #include "nsIVariant.h" +#include "mozilla/IOInterposer.h" #include "mozilla/Services.h" // How long we collect write oprerations @@ -279,9 +280,11 @@ void DOMStorageDBThread::ThreadFunc(void* aArg) { PR_SetCurrentThreadName("localStorage DB"); + mozilla::IOInterposer::RegisterCurrentThread(); DOMStorageDBThread* thread = static_cast(aArg); thread->ThreadFunc(); + mozilla::IOInterposer::UnregisterCurrentThread(); } void diff --git a/ipc/chromium/src/base/thread.cc b/ipc/chromium/src/base/thread.cc index c908f78a29e..968236327e0 100644 --- a/ipc/chromium/src/base/thread.cc +++ b/ipc/chromium/src/base/thread.cc @@ -9,6 +9,7 @@ #include "base/thread_local.h" #include "base/waitable_event.h" #include "GeckoProfiler.h" +#include "mozilla/IOInterposer.h" namespace base { @@ -139,6 +140,7 @@ void Thread::StopSoon() { void Thread::ThreadMain() { char aLocal; profiler_register_thread(name_.c_str(), &aLocal); + mozilla::IOInterposer::RegisterCurrentThread(); // The message loop for this thread. MessageLoop message_loop(startup_data_->options.message_loop_type); @@ -167,6 +169,7 @@ void Thread::ThreadMain() { // Assert that MessageLoop::Quit was called by ThreadQuitTask. DCHECK(GetThreadWasQuitProperly()); + mozilla::IOInterposer::UnregisterCurrentThread(); profiler_unregister_thread(); // We can't receive messages anymore. diff --git a/netwerk/cache2/CacheIOThread.cpp b/netwerk/cache2/CacheIOThread.cpp index 33c52defc85..e708dfff02d 100644 --- a/netwerk/cache2/CacheIOThread.cpp +++ b/netwerk/cache2/CacheIOThread.cpp @@ -9,6 +9,7 @@ #include "nsISupportsImpl.h" #include "nsPrintfCString.h" #include "nsThreadUtils.h" +#include "mozilla/IOInterposer.h" #include "mozilla/VisualEventTracer.h" namespace mozilla { @@ -152,8 +153,10 @@ already_AddRefed CacheIOThread::Target() void CacheIOThread::ThreadFunc(void* aClosure) { PR_SetCurrentThreadName("Cache2 I/O"); + mozilla::IOInterposer::RegisterCurrentThread(); CacheIOThread* thread = static_cast(aClosure); thread->ThreadFunc(); + mozilla::IOInterposer::UnregisterCurrentThread(); } void CacheIOThread::ThreadFunc() diff --git a/startupcache/StartupCache.cpp b/startupcache/StartupCache.cpp index b0ef64dbb49..923886c4609 100644 --- a/startupcache/StartupCache.cpp +++ b/startupcache/StartupCache.cpp @@ -7,6 +7,7 @@ #include "prio.h" #include "pldhash.h" #include "nsXPCOMStrings.h" +#include "mozilla/IOInterposer.h" #include "mozilla/MemoryReporting.h" #include "mozilla/scache/StartupCache.h" @@ -536,6 +537,7 @@ void StartupCache::ThreadedWrite(void *aClosure) { PR_SetCurrentThreadName("StartupCache"); + mozilla::IOInterposer::RegisterCurrentThread(); /* * It is safe to use the pointer passed in aClosure to reference the * StartupCache object because the thread's lifetime is tightly coupled to @@ -545,6 +547,7 @@ StartupCache::ThreadedWrite(void *aClosure) */ StartupCache* startupCacheObj = static_cast(aClosure); startupCacheObj->WriteToDisk(); + mozilla::IOInterposer::UnregisterCurrentThread(); } /* diff --git a/xpcom/build/IOInterposer.cpp b/xpcom/build/IOInterposer.cpp index b8ae223f5af..9ba52d60d46 100644 --- a/xpcom/build/IOInterposer.cpp +++ b/xpcom/build/IOInterposer.cpp @@ -9,6 +9,15 @@ #include "mozilla/Atomics.h" #include "mozilla/Mutex.h" +#if defined(MOZILLA_INTERNAL_API) +// We need to undefine MOZILLA_INTERNAL_API for RefPtr.h because IOInterposer +// does not clean up its data before shutdown. +#undef MOZILLA_INTERNAL_API +#include "mozilla/RefPtr.h" +#define MOZILLA_INTERNAL_API +#else +#include "mozilla/RefPtr.h" +#endif // defined(MOZILLA_INTERNAL_API) #include "mozilla/StaticPtr.h" #include "mozilla/ThreadLocal.h" #if !defined(XP_WIN) @@ -21,34 +30,38 @@ using namespace mozilla; namespace { +/** Find if a vector contains a specific element */ +template +bool VectorContains(const std::vector& vector, const T& element) +{ + return std::find(vector.begin(), vector.end(), element) != vector.end(); +} + +/** Remove element from a vector */ +template +void VectorRemove(std::vector& vector, const T& element) +{ + typename std::vector::iterator newEnd = std::remove(vector.begin(), + vector.end(), element); + vector.erase(newEnd, vector.end()); +} + /** Lists of Observers */ -struct ObserverLists { +struct ObserverLists : public AtomicRefCounted +{ ObserverLists() - : mObserverListsLock(PR_NewLock()) - , mIsEnabled(true) { - // We don't do MOZ_COUNT_CTOR(ObserverLists) as we will need to leak the - // IO interposer when doing late-write checks, which uses IO interposing - // to check for writes while static destructors are invoked. } - // mObserverListsLock guards access to lists of observers - // Note, we can use mozilla::Mutex here as the ObserverLists may be leaked, - // as we want to monitor IO during shutdown. Furthermore, as we may have to - // unregister observers during shutdown an OffTheBooksMutex is not an option - // either, as it base calls into sDeadlockDetector which may be nullptr - // during shutdown. - PRLock* mObserverListsLock; - - // Used for quickly disabling everything by IOInterposer::Disable() - mozilla::Atomic mIsEnabled; - - ~ObserverLists() + ObserverLists(ObserverLists const & aOther) + : mCreateObservers(aOther.mCreateObservers) + , mReadObservers(aOther.mReadObservers) + , mWriteObservers(aOther.mWriteObservers) + , mFSyncObservers(aOther.mFSyncObservers) + , mStatObservers(aOther.mStatObservers) + , mCloseObservers(aOther.mCloseObservers) { - PR_DestroyLock(mObserverListsLock); - mObserverListsLock = nullptr; } - // Lists of observers for read, write and fsync events respectively // These are implemented as vectors since they are allowed to survive gecko, // without reporting leaks. This is necessary for the IOInterposer to be used @@ -79,26 +92,271 @@ public: } }; +class PerThreadData +{ +public: + PerThreadData(bool aIsMainThread = false) + : mIsMainThread(aIsMainThread) + , mIsHandlingObservation(false) + , mCurrentGeneration(0) + { + } + + void + CallObservers(IOInterposeObserver::Observation& aObservation) + { + // Prevent recursive reporting. + if (mIsHandlingObservation) { + return; + } + + mIsHandlingObservation = true; + // Decide which list of observers to inform + std::vector* observers = nullptr; + switch (aObservation.ObservedOperation()) { + case IOInterposeObserver::OpCreateOrOpen: + { + observers = &mObserverLists->mCreateObservers; + } + break; + case IOInterposeObserver::OpRead: + { + observers = &mObserverLists->mReadObservers; + } + break; + case IOInterposeObserver::OpWrite: + { + observers = &mObserverLists->mWriteObservers; + } + break; + case IOInterposeObserver::OpFSync: + { + observers = &mObserverLists->mFSyncObservers; + } + break; + case IOInterposeObserver::OpStat: + { + observers = &mObserverLists->mStatObservers; + } + break; + case IOInterposeObserver::OpClose: + { + observers = &mObserverLists->mCloseObservers; + } + break; + default: + { + // Invalid IO operation, see documentation comment for + // IOInterposer::Report() + MOZ_ASSERT(false); + // Just ignore it in non-debug builds. + return; + } + } + MOZ_ASSERT(observers); + + // Inform observers + for (std::vector::iterator i = observers->begin(), + e = observers->end(); i != e; ++i) + { + (*i)->Observe(aObservation); + } + mIsHandlingObservation = false; + } + + inline uint32_t + GetCurrentGeneration() const + { + return mCurrentGeneration; + } + + inline bool + IsMainThread() const + { + return mIsMainThread; + } + + inline void + SetObserverLists(uint32_t aNewGeneration, RefPtr& aNewLists) + { + mCurrentGeneration = aNewGeneration; + mObserverLists = aNewLists; + } + +private: + bool mIsMainThread; + bool mIsHandlingObservation; + uint32_t mCurrentGeneration; + RefPtr mObserverLists; +}; + +class MasterList +{ +public: + MasterList() + : mLock(PR_NewLock()) + , mObservedOperations(IOInterposeObserver::OpNone) + , mIsEnabled(true) + { + } + + ~MasterList() + { + PR_DestroyLock(mLock); + mLock = nullptr; + } + + inline void + Disable() + { + mIsEnabled = false; + } + + void + Register(IOInterposeObserver::Operation aOp, IOInterposeObserver* aObserver) + { + AutoPRLock lock(mLock); + + ObserverLists* newLists = nullptr; + if (mObserverLists) { + newLists = new ObserverLists(*mObserverLists); + } else { + newLists = new ObserverLists(); + } + // You can register to observe multiple types of observations + // but you'll never be registered twice for the same observations. + if (aOp & IOInterposeObserver::OpCreateOrOpen && + !VectorContains(newLists->mCreateObservers, aObserver)) { + newLists->mCreateObservers.push_back(aObserver); + } + if (aOp & IOInterposeObserver::OpRead && + !VectorContains(newLists->mReadObservers, aObserver)) { + newLists->mReadObservers.push_back(aObserver); + } + if (aOp & IOInterposeObserver::OpWrite && + !VectorContains(newLists->mWriteObservers, aObserver)) { + newLists->mWriteObservers.push_back(aObserver); + } + if (aOp & IOInterposeObserver::OpFSync && + !VectorContains(newLists->mFSyncObservers, aObserver)) { + newLists->mFSyncObservers.push_back(aObserver); + } + if (aOp & IOInterposeObserver::OpStat && + !VectorContains(newLists->mStatObservers, aObserver)) { + newLists->mStatObservers.push_back(aObserver); + } + if (aOp & IOInterposeObserver::OpClose && + !VectorContains(newLists->mCloseObservers, aObserver)) { + newLists->mCloseObservers.push_back(aObserver); + } + mObserverLists = newLists; + mObservedOperations = (IOInterposeObserver::Operation) + (mObservedOperations | aOp); + + mCurrentGeneration++; + } + + void + Unregister(IOInterposeObserver::Operation aOp, IOInterposeObserver* aObserver) + { + AutoPRLock lock(mLock); + + ObserverLists* newLists = nullptr; + if (mObserverLists) { + newLists = new ObserverLists(*mObserverLists); + } else { + newLists = new ObserverLists(); + } + + if (aOp & IOInterposeObserver::OpCreateOrOpen) { + VectorRemove(newLists->mCreateObservers, aObserver); + if (newLists->mCreateObservers.empty()) { + mObservedOperations = (IOInterposeObserver::Operation) + (mObservedOperations & + ~IOInterposeObserver::OpCreateOrOpen); + } + } + if (aOp & IOInterposeObserver::OpRead) { + VectorRemove(newLists->mReadObservers, aObserver); + if (newLists->mReadObservers.empty()) { + mObservedOperations = (IOInterposeObserver::Operation) + (mObservedOperations & ~IOInterposeObserver::OpRead); + } + } + if (aOp & IOInterposeObserver::OpWrite) { + VectorRemove(newLists->mWriteObservers, aObserver); + if (newLists->mWriteObservers.empty()) { + mObservedOperations = (IOInterposeObserver::Operation) + (mObservedOperations & ~IOInterposeObserver::OpWrite); + } + } + if (aOp & IOInterposeObserver::OpFSync) { + VectorRemove(newLists->mFSyncObservers, aObserver); + if (newLists->mFSyncObservers.empty()) { + mObservedOperations = (IOInterposeObserver::Operation) + (mObservedOperations & ~IOInterposeObserver::OpFSync); + } + } + if (aOp & IOInterposeObserver::OpStat) { + VectorRemove(newLists->mStatObservers, aObserver); + if (newLists->mStatObservers.empty()) { + mObservedOperations = (IOInterposeObserver::Operation) + (mObservedOperations & ~IOInterposeObserver::OpStat); + } + } + if (aOp & IOInterposeObserver::OpClose) { + VectorRemove(newLists->mCloseObservers, aObserver); + if (newLists->mCloseObservers.empty()) { + mObservedOperations = (IOInterposeObserver::Operation) + (mObservedOperations & ~IOInterposeObserver::OpClose); + } + } + mObserverLists = newLists; + mCurrentGeneration++; + } + + void + Update(PerThreadData &aPtd) + { + if (mCurrentGeneration == aPtd.GetCurrentGeneration()) { + return; + } + // If the generation counts don't match then we need to update the current + // thread's observer list with the new master list. + AutoPRLock lock(mLock); + aPtd.SetObserverLists(mCurrentGeneration, mObserverLists); + } + + inline bool + IsObservedOperation(IOInterposeObserver::Operation aOp) + { + // The quick reader may observe that no locks are being employed here, + // hence the result of the operations is truly undefined. However, most + // computers will usually return either true or false, which is good enough. + // It is not a problem if we occasionally report more or less IO than is + // actually occurring. + return mIsEnabled && !!(mObservedOperations & aOp); + } + +private: + RefPtr mObserverLists; + // Note, we cannot use mozilla::Mutex here as the ObserverLists may be leaked + // (We want to monitor IO during shutdown). Furthermore, as we may have to + // unregister observers during shutdown an OffTheBooksMutex is not an option + // either, as its base calls into sDeadlockDetector which may be nullptr + // during shutdown. + PRLock* mLock; + // Flags tracking which operations are being observed + IOInterposeObserver::Operation mObservedOperations; + // Used for quickly disabling everything by IOInterposer::Disable() + Atomic mIsEnabled; + // Used to inform threads that the master observer list has changed + Atomic mCurrentGeneration; +}; + // List of observers registered -static StaticAutoPtr sObserverLists; -static ThreadLocal sIsMainThread; - -/** Find if a vector contains a specific element */ -template -bool VectorContains(const std::vector& vector, const T& element) -{ - return std::find(vector.begin(), vector.end(), element) != vector.end(); -} - -/** Remove element from a vector */ -template -void VectorRemove(std::vector& vector, const T& element) -{ - typename std::vector::iterator newEnd = std::remove(vector.begin(), - vector.end(), element); - vector.erase(newEnd, vector.end()); -} - +static StaticAutoPtr sMasterList; +static ThreadLocal sThreadLocalData; } // anonymous namespace IOInterposeObserver::Observation::Observation(Operation aOperation, @@ -135,27 +393,25 @@ IOInterposeObserver::Observation::Report() } } -// Flags tracking which operations are being observed -IOInterposeObserver::Operation IOInterposer::sObservedOperations = - IOInterposeObserver::OpNone; - -/* static */ void IOInterposer::Init() +/* static */ bool +IOInterposer::Init() { // Don't initialize twice... - if (sObserverLists) { - return; + if (sMasterList) { + return true; + } + if (!sThreadLocalData.init()) { + return false; } - sObserverLists = new ObserverLists(); - sObservedOperations = IOInterposeObserver::OpNone; - if (sIsMainThread.init()) { #if defined(XP_WIN) - bool isMainThread = XRE_GetWindowsEnvironment() != - WindowsEnvironmentType_Metro; + bool isMainThread = XRE_GetWindowsEnvironment() != + WindowsEnvironmentType_Metro; #else - bool isMainThread = true; + bool isMainThread = true; #endif - sIsMainThread.set(isMainThread); - } + RegisterCurrentThread(isMainThread); + sMasterList = new MasterList(); + // Now we initialize the various interposers depending on platform InitPoisonIOInterposer(); // We don't hook NSPR on Windows because PoisonIOInterposer captures a @@ -163,225 +419,111 @@ IOInterposeObserver::Operation IOInterposer::sObservedOperations = #if !defined(XP_WIN) InitNSPRIOInterposing(); #endif + return true; } /* static */ bool IOInterposeObserver::IsMainThread() { - return sIsMainThread.initialized() && sIsMainThread.get(); + if (!sThreadLocalData.initialized()) { + return false; + } + PerThreadData *ptd = sThreadLocalData.get(); + if (!ptd) { + return false; + } + return ptd->IsMainThread(); } -/* static */ void IOInterposer::Clear() +/* static */ void +IOInterposer::Clear() { - // Clear() shouldn't be called if Init() wasn't called, - MOZ_ASSERT(sObserverLists); - if (sObserverLists) { - // We require everybody unregister before clearing. If somebody didn't then - // this is probably a case where one consumer clears the IO interposer and - // another consumer still wants events. - MOZ_ASSERT(sObserverLists->mReadObservers.empty()); - MOZ_ASSERT(sObserverLists->mWriteObservers.empty()); - MOZ_ASSERT(sObserverLists->mFSyncObservers.empty()); - - sObserverLists = nullptr; - sObservedOperations = IOInterposeObserver::OpNone; - } + sMasterList = nullptr; } /* static */ void IOInterposer::Disable() { - if (!sObserverLists) { + if (!sMasterList) { return; } - sObserverLists->mIsEnabled = false; + sMasterList->Disable(); } -/* static */ void IOInterposer::Report( - IOInterposeObserver::Observation& aObservation) +/* static */ void +IOInterposer::Report(IOInterposeObserver::Observation& aObservation) { - // IOInterposer::Init most be called before this method - MOZ_ASSERT(sObserverLists); - if (!sObserverLists) { + MOZ_ASSERT(sMasterList); + if (!sMasterList) { return; } - //TODO: We only need read access here, so we should investigate the - // performance overhead involved in using some kind of shared lock. - // Work towards this end is tracked in bug #913653 - AutoPRLock listLock(sObserverLists->mObserverListsLock); + PerThreadData* ptd = sThreadLocalData.get(); + if (!ptd) { + // In this case the current thread is not registered with IOInterposer. + // Alternatively we could take the slow path and just lock everything if + // we're not registered. That could potentially perform poorly, though. + return; + } + sMasterList->Update(*ptd); - // Don't try to report if there's nobody listening + // Don't try to report if there's nobody listening. if (!IOInterposer::IsObservedOperation(aObservation.ObservedOperation())) { return; } - // Decide which list of observers to inform - std::vector* observers = nullptr; - switch (aObservation.ObservedOperation()) { - case IOInterposeObserver::OpCreateOrOpen: - { - observers = &sObserverLists->mCreateObservers; - } - break; - case IOInterposeObserver::OpRead: - { - observers = &sObserverLists->mReadObservers; - } - break; - case IOInterposeObserver::OpWrite: - { - observers = &sObserverLists->mWriteObservers; - } - break; - case IOInterposeObserver::OpFSync: - { - observers = &sObserverLists->mFSyncObservers; - } - break; - case IOInterposeObserver::OpStat: - { - observers = &sObserverLists->mStatObservers; - } - break; - case IOInterposeObserver::OpClose: - { - observers = &sObserverLists->mCloseObservers; - } - break; - default: - { - // Invalid IO operation, see documentation comment for Report() - MOZ_ASSERT(false); - // Just ignore is in non-debug builds. - return; - } - } - MOZ_ASSERT(observers); - - // Inform observers - uint32_t nObservers = observers->size(); - for (uint32_t i = 0; i < nObservers; ++i) { - (*observers)[i]->Observe(aObservation); - } + ptd->CallObservers(aObservation); } /* static */ bool IOInterposer::IsObservedOperation(IOInterposeObserver::Operation aOp) { - return sObserverLists && sObserverLists->mIsEnabled && - !!(sObservedOperations & aOp); + return sMasterList && sMasterList->IsObservedOperation(aOp); } -/* static */ void IOInterposer::Register(IOInterposeObserver::Operation aOp, - IOInterposeObserver* aObserver) +/* static */ void +IOInterposer::Register(IOInterposeObserver::Operation aOp, + IOInterposeObserver* aObserver) { - // We should never register nullptr as observer MOZ_ASSERT(aObserver); - if (!sObserverLists || !aObserver) { + if (!sMasterList || !aObserver) { return; } - AutoPRLock listLock(sObserverLists->mObserverListsLock); - - // You can register to observe multiple types of observations - // but you'll never be registered twice for the same observations. - if (aOp & IOInterposeObserver::OpCreateOrOpen && - !VectorContains(sObserverLists->mCreateObservers, aObserver)) { - sObserverLists->mCreateObservers.push_back(aObserver); - } - if (aOp & IOInterposeObserver::OpRead && - !VectorContains(sObserverLists->mReadObservers, aObserver)) { - sObserverLists->mReadObservers.push_back(aObserver); - } - if (aOp & IOInterposeObserver::OpWrite && - !VectorContains(sObserverLists->mWriteObservers, aObserver)) { - sObserverLists->mWriteObservers.push_back(aObserver); - } - if (aOp & IOInterposeObserver::OpFSync && - !VectorContains(sObserverLists->mFSyncObservers, aObserver)) { - sObserverLists->mFSyncObservers.push_back(aObserver); - } - if (aOp & IOInterposeObserver::OpStat && - !VectorContains(sObserverLists->mStatObservers, aObserver)) { - sObserverLists->mStatObservers.push_back(aObserver); - } - if (aOp & IOInterposeObserver::OpClose && - !VectorContains(sObserverLists->mCloseObservers, aObserver)) { - sObserverLists->mCloseObservers.push_back(aObserver); - } - - // Update field of observed operation with the operations that the new - // observer is observing. - sObservedOperations = (IOInterposeObserver::Operation) - (sObservedOperations | aOp); + sMasterList->Register(aOp, aObserver); } -/* static */ void IOInterposer::Unregister(IOInterposeObserver::Operation aOp, - IOInterposeObserver* aObserver) +/* static */ void +IOInterposer::Unregister(IOInterposeObserver::Operation aOp, + IOInterposeObserver* aObserver) { - if (!sObserverLists) { + if (!sMasterList) { return; } - AutoPRLock listLock(sObserverLists->mObserverListsLock); - - if (aOp & IOInterposeObserver::OpCreateOrOpen) { - VectorRemove(sObserverLists->mCreateObservers, aObserver); - if (sObserverLists->mCreateObservers.empty()) { - sObservedOperations = (IOInterposeObserver::Operation) - (sObservedOperations & - ~IOInterposeObserver::OpCreateOrOpen); - } - } - if (aOp & IOInterposeObserver::OpRead) { - VectorRemove(sObserverLists->mReadObservers, aObserver); - if (sObserverLists->mReadObservers.empty()) { - sObservedOperations = (IOInterposeObserver::Operation) - (sObservedOperations & ~IOInterposeObserver::OpRead); - } - } - if (aOp & IOInterposeObserver::OpWrite) { - VectorRemove(sObserverLists->mWriteObservers, aObserver); - if (sObserverLists->mWriteObservers.empty()) { - sObservedOperations = (IOInterposeObserver::Operation) - (sObservedOperations & ~IOInterposeObserver::OpWrite); - } - } - if (aOp & IOInterposeObserver::OpFSync) { - VectorRemove(sObserverLists->mFSyncObservers, aObserver); - if (sObserverLists->mFSyncObservers.empty()) { - sObservedOperations = (IOInterposeObserver::Operation) - (sObservedOperations & ~IOInterposeObserver::OpFSync); - } - } - if (aOp & IOInterposeObserver::OpStat) { - VectorRemove(sObserverLists->mStatObservers, aObserver); - if (sObserverLists->mStatObservers.empty()) { - sObservedOperations = (IOInterposeObserver::Operation) - (sObservedOperations & ~IOInterposeObserver::OpStat); - } - } - if (aOp & IOInterposeObserver::OpClose) { - VectorRemove(sObserverLists->mCloseObservers, aObserver); - if (sObserverLists->mCloseObservers.empty()) { - sObservedOperations = (IOInterposeObserver::Operation) - (sObservedOperations & ~IOInterposeObserver::OpClose); - } - } + sMasterList->Unregister(aOp, aObserver); } /* static */ void IOInterposer::RegisterCurrentThread(bool aIsMainThread) { - // Right now this is a no-op unless we're running on Metro. - // More cross-platform stuff will be added in the near future, stay tuned! -#if defined(XP_WIN) - if (XRE_GetWindowsEnvironment() != WindowsEnvironmentType_Metro || - !sIsMainThread.initialized()) { + if (!sThreadLocalData.initialized()) { return; } - sIsMainThread.set(aIsMainThread); -#endif + MOZ_ASSERT(!sThreadLocalData.get()); + PerThreadData* curThreadData = new PerThreadData(aIsMainThread); + sThreadLocalData.set(curThreadData); +} + +/* static */ void +IOInterposer::UnregisterCurrentThread() +{ + if (!sThreadLocalData.initialized()) { + return; + } + PerThreadData* curThreadData = sThreadLocalData.get(); + MOZ_ASSERT(curThreadData); + sThreadLocalData.set(nullptr); + delete curThreadData; } diff --git a/xpcom/build/IOInterposer.h b/xpcom/build/IOInterposer.h index e73ec1c0fd6..f6df8101e16 100644 --- a/xpcom/build/IOInterposer.h +++ b/xpcom/build/IOInterposer.h @@ -161,9 +161,6 @@ protected: */ class IOInterposer MOZ_FINAL { - // Track whether or not a given type of observation is being observed - static IOInterposeObserver::Operation sObservedOperations; - // No instance of class should be created, they'd be empty anyway. IOInterposer(); public: @@ -178,7 +175,7 @@ public: * Remark, it's safe to call this method multiple times, so just call it when * you to utilize IO interposing. */ - static void Init(); + static bool Init(); /** * This function must be called from the main thread, and furthermore @@ -249,13 +246,26 @@ public: IOInterposeObserver* aObserver); /** - * Registers the current thread with the IOInterposer. + * Registers the current thread with the IOInterposer. This must be done to + * ensure that per-thread data is created in an orderly fashion. + * We could have written this to initialize that data lazily, however this + * could have unintended consequences if a thread that is not aware of + * IOInterposer was implicitly registered: its per-thread data would never + * be deleted because it would not know to unregister itself. * * @param aIsMainThread true if IOInterposer should treat the current thread * as the main thread. */ static void RegisterCurrentThread(bool aIsMainThread = false); + + /** + * Unregisters the current thread with the IOInterposer. This is important + * to call when a thread is shutting down because it cleans up data that + * is stored in a TLS slot. + */ + static void + UnregisterCurrentThread(); }; class IOInterposerInit diff --git a/xpcom/threads/nsThread.cpp b/xpcom/threads/nsThread.cpp index 017ecbdfa86..3c3f09c08f9 100644 --- a/xpcom/threads/nsThread.cpp +++ b/xpcom/threads/nsThread.cpp @@ -24,6 +24,7 @@ #include "prlog.h" #include "nsIObserverService.h" #include "mozilla/HangMonitor.h" +#include "mozilla/IOInterposer.h" #include "mozilla/Services.h" #include "nsXPCOMPrivate.h" #include "mozilla/ChaosMode.h" @@ -291,6 +292,8 @@ nsThread::ThreadFunc(void *arg) // Inform the ThreadManager nsThreadManager::get()->RegisterCurrentThread(self); + mozilla::IOInterposer::RegisterCurrentThread(); + // Wait for and process startup event nsCOMPtr event; if (!self->GetEvent(true, getter_AddRefs(event))) { @@ -328,6 +331,8 @@ nsThread::ThreadFunc(void *arg) } } + mozilla::IOInterposer::UnregisterCurrentThread(); + // Inform the threadmanager that this thread is going away nsThreadManager::get()->UnregisterCurrentThread(self);