diff --git a/modules/libpref/Preferences.cpp b/modules/libpref/Preferences.cpp index 7875044379b6..40c76fc5ba77 100644 --- a/modules/libpref/Preferences.cpp +++ b/modules/libpref/Preferences.cpp @@ -3039,36 +3039,27 @@ class PreferencesWriter final { } static void Flush() { - MOZ_DIAGNOSTIC_ASSERT(sPendingWriteCount >= 0); - // SpinEventLoopUntil is unfortunate, but ultimately it's the best thing - // we can do here given the constraint that we need to ensure that - // the preferences on disk match what we have in memory. We could - // easily perform the write here ourselves by doing exactly what - // happens in PWRunnable::Run. This would be the right thing to do - // if we're stuck here because other unrelated runnables are taking - // a long time, and the wrong thing to do if PreferencesWriter::Write - // is what takes a long time, as we would be trading a SpinEventLoopUntil - // for a synchronous disk write, wherein we could not even spin the - // event loop. Given that PWRunnable generally runs on a thread pool, - // if we're stuck here, it's likely because of PreferencesWriter::Write - // and not some other runnable. Thus, spin away. - mozilla::SpinEventLoopUntil([]() { return sPendingWriteCount <= 0; }); + // This can be further optimized; instead of waiting for all of the writer + // thread to be available, we just have to wait for all the pending writes + // to be done. + if (!sPendingWriteData.compareExchange(nullptr, nullptr)) { + nsresult rv = NS_OK; + nsCOMPtr target = + do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID, &rv); + if (NS_SUCCEEDED(rv)) { + target->Dispatch(NS_NewRunnableFunction("Preferences_dummy", [] {}), + nsIEventTarget::DISPATCH_SYNC); + } + } } // This is the data that all of the runnables (see below) will attempt // to write. It will always have the most up to date version, or be // null, if the up to date information has already been written out. static Atomic sPendingWriteData; - - // This is the number of writes via PWRunnables which have been dispatched - // but not yet completed. This is intended to be used by Flush to ensure - // that there are no outstanding writes left incomplete, and thus our prefs - // on disk are in sync with what we have in memory. - static Atomic sPendingWriteCount; }; Atomic PreferencesWriter::sPendingWriteData(nullptr); -Atomic PreferencesWriter::sPendingWriteCount(0); class PWRunnable : public Runnable { public: @@ -3098,13 +3089,6 @@ class PWRunnable : public Runnable { } })); } - - // We've completed the write to the best of our abilities, whether - // we had prefs to write or another runnable got to them first. If - // PreferencesWriter::Write failed, this is still correct as the - // write is no longer outstanding, and the above HandleDirty call - // will just start the cycle again. - PreferencesWriter::sPendingWriteCount--; return rv; } @@ -4137,17 +4121,6 @@ nsresult Preferences::WritePrefFile(nsIFile* aFile, SaveMethod aSaveMethod) { do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID, &rv); if (NS_SUCCEEDED(rv)) { bool async = aSaveMethod == SaveMethod::Asynchronous; - - // Increment sPendingWriteCount, even though it's redundant to track this - // in the case of a sync runnable; it just makes it easier to simply - // decrement this inside PWRunnable. We could alternatively increment - // sPendingWriteCount in PWRunnable's constructor, but if for any reason - // in future code we create a PWRunnable without dispatching it, we would - // get stuck in an infinite SpinEventLoopUntil inside - // PreferencesWriter::Flush. Better that in future code we miss an - // increment of sPendingWriteCount and cause a simple crash due to it - // ending up negative. - PreferencesWriter::sPendingWriteCount++; if (async) { rv = target->Dispatch(new PWRunnable(aFile), nsIEventTarget::DISPATCH_NORMAL); diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index e0c45a321892..1c909fa5ca2e 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -8462,11 +8462,11 @@ #ifdef MOZ_CODE_COVERAGE value: 0 #else - #ifdef NIGHTLY_BUILD +# ifdef NIGHTLY_BUILD value: 3 - #else +# else value: 1 - #endif +# endif #endif mirror: always @@ -8475,7 +8475,7 @@ - name: toolkit.shutdown.fastShutdownStage type: int32_t #if defined(NIGHTLY_BUILD) && !defined(DEBUG) && !defined(MOZ_CODE_COVERAGE) && !defined(MOZ_VALGRIND) && !defined(MOZ_PROFILE_GENERATE) - value: 1 + value: 2 #else value: 0 #endif diff --git a/xpcom/build/IOInterposer.cpp b/xpcom/build/IOInterposer.cpp index 8815c7aa6a6a..8a8343f3fd24 100644 --- a/xpcom/build/IOInterposer.cpp +++ b/xpcom/build/IOInterposer.cpp @@ -290,10 +290,11 @@ class MasterList { } inline bool IsObservedOperation(mozilla::IOInterposeObserver::Operation aOp) { - // This does not occur inside of a lock, so this makes no guarantees that - // the observers are in sync with this. That is acceptable; it is not a - // problem if we occasionally report more or less IO than is actually - // occurring. + // 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); } @@ -306,9 +307,7 @@ class MasterList { // during shutdown. mozilla::IOInterposer::Mutex mLock; // Flags tracking which operations are being observed - mozilla::Atomic - mObservedOperations; + mozilla::IOInterposeObserver::Operation mObservedOperations; // Used for quickly disabling everything by IOInterposer::Disable() mozilla::Atomic mIsEnabled; // Used to inform threads that the master observer list has changed