Bug 1372988 part A - Warn when user prefs are set after the profile is dead, because the prefs won't get saved anyway. r?milan draft
authorBenjamin Smedberg <benjamin@smedbergs.us>
Wed, 21 Jun 2017 11:17:01 -0400
changeset 598555 854189288495ba6527e22b41d02b4b812f2a5da2
parent 598554 3990cc17639d7aaf08b3dcb909a83d49550b7158
child 598556 ab7d4f061efda6fce640297f85cd59458f36b9c0
child 598557 87a6d39edf0a4b8d93311163b20509eb14cad656
push id65242
push userbsmedberg@mozilla.com
push dateWed, 21 Jun 2017 20:34:07 +0000
reviewersmilan
bugs1372988
milestone56.0a1
Bug 1372988 part A - Warn when user prefs are set after the profile is dead, because the prefs won't get saved anyway. r?milan MozReview-Commit-ID: IfwL5yYtLcF
modules/libpref/Preferences.cpp
modules/libpref/Preferences.h
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -97,16 +97,19 @@ Preferences::DirtyCallback()
   if (!XRE_IsParentProcess()) {
     // TODO: this should really assert because you can't set prefs in a
     // content process. But so much code currently does this that we just
     // ignore it for now.
     return;
   }
   if (gHashTable && sPreferences && !sPreferences->mDirty) {
     sPreferences->mDirty = true;
+
+    NS_WARNING_ASSERTION(!sPreferences->mProfileShutdown,
+                         "Setting user pref after profile shutdown.");
   }
 }
 
 // Prototypes
 static nsresult openPrefFile(nsIFile* aFile);
 static nsresult pref_InitInitialObjects(void);
 static nsresult pref_LoadPrefsInDirList(const char *listId);
 static nsresult ReadExtensionPrefs(nsIFile *aFile);
@@ -791,21 +794,29 @@ Preferences::Observe(nsISupports *aSubje
 {
   if (MOZ_UNLIKELY(!XRE_IsParentProcess())) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   nsresult rv = NS_OK;
 
   if (!nsCRT::strcmp(aTopic, "profile-before-change")) {
-    rv = SavePrefFile(nullptr);
-  } else if (!nsCRT::strcmp(aTopic, "profile-before-change-telemetry")) {
+    // Normally prefs aren't written after this point, and so we kick off
+    // an asynchronous pref save so that I/O can be done in parallel with
+    // other shutdown.
     if (AllowOffMainThreadSave()) {
-      PreferencesWriter::Flush();
+      SavePrefFile(nullptr);
     }
+  } else if (!nsCRT::strcmp(aTopic, "profile-before-change-telemetry")) {
+    // It's possible that a profile-before-change observer after ours
+    // set a pref. A blocking save here re-saves if necessary and also waits
+    // for any pending saves to complete.
+    SavePrefFileBlocking();
+    MOZ_ASSERT(!mDirty, "Preferences should not be dirty");
+    mProfileShutdown = true;
   } else if (!strcmp(aTopic, "load-extension-defaults")) {
     pref_LoadPrefsInDirList(NS_EXT_PREFS_DEFAULTS_DIR_LIST);
   } else if (!nsCRT::strcmp(aTopic, "reload-default-prefs")) {
     // Reload the default prefs from file.
     pref_InitInitialObjects();
   } else if (!nsCRT::strcmp(aTopic, "suspend_process_notification")) {
     // Our process is being suspended. The OS may wake our process later,
     // or it may kill the process. In case our process is going to be killed
@@ -1149,16 +1160,23 @@ Preferences::SavePrefFileInternal(nsIFil
     }
 
     // the mDirty flag tells us if we should write to mCurrentFile
     // we only check this flag when the caller wants to write to the default
     if (!mDirty) {
       return NS_OK;
     }
 
+    // check for profile shutdown after mDirty because the runnables from
+    // DirtyCallback can still be pending
+    if (mProfileShutdown) {
+      NS_WARNING("Cannot save pref file after profile shutdown.");
+      return NS_ERROR_ILLEGAL_DURING_SHUTDOWN;
+    }
+
     // It's possible that we never got a prefs file.
     nsresult rv = NS_OK;
     if (mCurrentFile) {
       rv = WritePrefFile(mCurrentFile, aSaveMethod);
     }
 
     // If we succeeded writing to mCurrentFile, reset the dirty flag
     if (NS_SUCCEEDED(rv)) {
--- a/modules/libpref/Preferences.h
+++ b/modules/libpref/Preferences.h
@@ -481,16 +481,17 @@ protected:
   static nsresult RegisterCallbackAndCall(PrefChangedFunc aCallback,
                                           const char* aPref,
                                           void* aClosure,
                                           MatchKind aMatchKind);
 
 private:
   nsCOMPtr<nsIFile>        mCurrentFile;
   bool                     mDirty;
+  bool                     mProfileShutdown = false;
 
   static Preferences*      sPreferences;
   static nsIPrefBranch*    sRootBranch;
   static nsIPrefBranch*    sDefaultRootBranch;
   static bool              sShutdown;
 
   /**
    * Init static members.  TRUE if it succeeded.  Otherwise, FALSE.