Bug 885096 - hasMutex should not be called from a lazy getter. r=bbondy

toolkit/mozapps/update/nsUpdateService.js

@@ -196,7 +196,8 @@ const PING_BGUC_INVALID_DEFAULT_URL          = 2;
 const PING_BGUC_INVALID_CUSTOM_URL           = 3;
 // Invalid url for app.update.url.override user preference (no notification)
 const PING_BGUC_INVALID_OVERRIDE_URL         = 4;
-// Unable to check for updates per gCanCheckForUpdates (no notification)
+// Unable to check for updates per gCanCheckForUpdates and hasUpdateMutex()
+// (no notification)
 const PING_BGUC_UNABLE_TO_CHECK              = 5;
 // Already has an active update in progress (no notification)
 const PING_BGUC_HAS_ACTIVEUPDATE             = 6;
@@ -258,13 +259,14 @@ const PING_BGUC_ADDON_UPDATES_FOR_INCOMPAT   = 29;
 const PING_BGUC_ADDON_HAVE_INCOMPAT          = 30;
 
 var gLocale = null;
+var gUpdateMutexHandle = null;
 
 #ifdef MOZ_WIDGET_GONK
+var gSDCardMountLock = null;
+
 XPCOMUtils.defineLazyGetter(this, "gExtStorage", function aus_gExtStorage() {
     return Services.env.get("EXTERNAL_STORAGE");
 });
-
-var gSDCardMountLock = null;
 #endif
 
 XPCOMUtils.defineLazyModuleGetter(this, "UpdateChannel",
@@ -483,10 +485,13 @@ function closeHandle(handle) {
 /**
  * Creates a mutex.
  *
- * @param aAllowExisting false if the function should fail if the mutex exists
+ * @param  aName
- * @return The Win32 handle to the mutex
+ *         The name for the mutex.
+ * @param  aAllowExisting
+ *         If false the function will close the handle and return null.
+ * @return The Win32 handle to the mutex.
  */
-function createMutex(name, aAllowExisting) {
+function createMutex(aName, aAllowExisting) {
   if (aAllowExisting === undefined) {
     aAllowExisting = true;
   }
@@ -501,7 +506,7 @@ function createMutex(name, aAllowExisting) {
                                  ctypes.int32_t, /* initial owner */
                                  ctypes.jschar.ptr); /* name */
 
-  var handle = CreateMutexW(null, INITIAL_OWN, name);
+  var handle = CreateMutexW(null, INITIAL_OWN, aName);
   var alreadyExists = ctypes.winLastError == ERROR_ALREADY_EXISTS;
   if (handle && !handle.isNull() && !aAllowExisting && alreadyExists) {
     closeHandle(handle);
@@ -554,11 +559,11 @@ function getPerInstallationMutexName(aGlobal) {
  */
 function hasUpdateMutex() {
 #ifdef XP_WIN
-  if (!this._updateMutexHandle) {
+  if (!gUpdateMutexHandle) {
-    this._updateMutexHandle = createMutex(getPerInstallationMutexName(true), false);
+    gUpdateMutexHandle = createMutex(getPerInstallationMutexName(true), false);
   }
 
-  return !!this._updateMutexHandle;
+  return !!gUpdateMutexHandle;
 #else
   return true;
 #endif // XP_WIN
@@ -669,13 +674,6 @@ XPCOMUtils.defineLazyGetter(this, "gCanApplyUpdates", function aus_gCanApplyUpda
     }
   } // if (!useService)
 
-  if (!hasUpdateMutex()) {
-    LOG("gCanApplyUpdates - unable to apply updates because another instance" +
-        "of the application is already handling updates for this " +
-        "installation.");
-    return false;
-  }
-
   LOG("gCanApplyUpdates - able to apply updates");
   submitHasPermissionsTelemetryPing(true);
   return true;
@@ -795,13 +793,6 @@ XPCOMUtils.defineLazyGetter(this, "gCanCheckForUpdates", function aus_gCanCheckF
     return false;
   }
 
-  if (!hasUpdateMutex()) {
-    LOG("gCanCheckForUpdates - unable to apply updates because another " +
-        "instance of the application is already handling updates for this " +
-        "installation.");
-    return false;
-  }
-
   LOG("gCanCheckForUpdates - able to check for updates");
   return true;
 });
@@ -2611,7 +2602,7 @@ UpdateService.prototype = {
       else if (!getPref("getBoolPref", PREF_APP_UPDATE_ENABLED, true)) {
         this._backgroundUpdateCheckCodePing(PING_BGUC_PREF_DISABLED);
       }
-      else if (!gCanCheckForUpdates) {
+      else if (!(gCanCheckForUpdates && hasUpdateMutex())) {
         this._backgroundUpdateCheckCodePing(PING_BGUC_UNABLE_TO_CHECK);
       }
       else if (!this.backgroundChecker._enabled) {
@@ -2759,7 +2750,7 @@ UpdateService.prototype = {
       return;
     }
 
-    if (!gCanApplyUpdates) {
+    if (!(gCanApplyUpdates && hasUpdateMutex())) {
       LOG("UpdateService:_selectAndInstallUpdate - the user is unable to " +
           "apply updates... prompting");
       this._showPrompt(update);
@@ -2970,7 +2961,8 @@ UpdateService.prototype = {
     if (--this._updateCheckCount > 0)
       return;
 
-    if (this._incompatibleAddons.length > 0 || !gCanApplyUpdates) {
+    if (this._incompatibleAddons.length > 0 ||
+        !(gCanApplyUpdates && hasUpdateMutex())) {
       LOG("UpdateService:onUpdateEnded - prompting because there are " +
           "incompatible add-ons");
       this._showPrompt(this._update);
@@ -3005,14 +2997,14 @@ UpdateService.prototype = {
    * See nsIUpdateService.idl
    */
   get canCheckForUpdates() {
-    return gCanCheckForUpdates;
+    return gCanCheckForUpdates && hasUpdateMutex();
   },
 
   /**
    * See nsIUpdateService.idl
    */
   get canApplyUpdates() {
-    return gCanApplyUpdates;
+    return gCanApplyUpdates && hasUpdateMutex();
   },
 
   /**
@@ -3798,7 +3790,7 @@ Checker.prototype = {
     }
 
     return getPref("getBoolPref", PREF_APP_UPDATE_ENABLED, true) &&
-           gCanCheckForUpdates && this._enabled;
+           gCanCheckForUpdates && hasUpdateMutex() && this._enabled;
   },
 
   /**

toolkit/mozapps/update/tests/unit_aus_update/canCheckForAndCanApplyUpdates.js

@@ -8,21 +8,127 @@ function run_test() {
 
   // Verify write access to the custom app dir
   logTestInfo("testing write access to the application directory");
-  var testFile = getCurrentProcessDir();
+  let testFile = getCurrentProcessDir();
   testFile.append("update_write_access_test");
-  testFile.create(AUS_Ci.nsIFile.NORMAL_FILE_TYPE, 0o644);
+  testFile.create(AUS_Ci.nsIFile.NORMAL_FILE_TYPE, PERMS_FILE);
   do_check_true(testFile.exists());
   testFile.remove(false);
   do_check_false(testFile.exists());
 
   standardInit();
 
+  if (IS_WIN) {
+    // Create a mutex to prevent being able to check for or apply updates.
+    logTestInfo("attempting to create mutex");
+    let handle = createMutex(getPerInstallationMutexName());
+
+    logTestInfo("testing that the mutex was successfully created");
+    do_check_neq(handle, null);
+
+    // Check if available updates cannot be checked for when there is a mutex
+    // for this installation.
+    logTestInfo("testing nsIApplicationUpdateService:canCheckForUpdates is " +
+                "false when there is a mutex");
+    do_check_false(gAUS.canCheckForUpdates);
+
+    // Check if updates cannot be applied when there is a mutex for this
+    // installation.
+    logTestInfo("testing nsIApplicationUpdateService:canApplyUpdates is " +
+                "false when there is a mutex");
+    do_check_false(gAUS.canApplyUpdates);
+
+    logTestInfo("destroying mutex");
+    closeHandle(handle)
+  }
+
   // Check if available updates can be checked for
-  logTestInfo("testing nsIApplicationUpdateService:canCheckForUpdates");
+  logTestInfo("testing nsIApplicationUpdateService:canCheckForUpdates is true");
   do_check_true(gAUS.canCheckForUpdates);
   // Check if updates can be applied
-  logTestInfo("testing nsIApplicationUpdateService:canApplyUpdates");
+  logTestInfo("testing nsIApplicationUpdateService:canApplyUpdates is true");
   do_check_true(gAUS.canApplyUpdates);
 
+  if (IS_WIN) {
+    // Attempt to create a mutex when application update has already created one
+    // with the same name.
+    logTestInfo("attempting to create mutex");
+    let handle = createMutex(getPerInstallationMutexName());
+
+    logTestInfo("testing that the mutex was not successfully created");
+    do_check_eq(handle, null);
+  }
+
   doTestFinish();
 }
+
+if (IS_WIN) {
+  /**
+   * Determines a unique mutex name for the installation.
+   *
+   * @return Global mutex path.
+   */
+  function getPerInstallationMutexName() {
+    let hasher = AUS_Cc["@mozilla.org/security/hash;1"].
+                 createInstance(AUS_Ci.nsICryptoHash);
+    hasher.init(hasher.SHA1);
+
+    let exeFile = Services.dirsvc.get(XRE_EXECUTABLE_FILE, AUS_Ci.nsILocalFile);
+
+    let converter = AUS_Cc["@mozilla.org/intl/scriptableunicodeconverter"].
+                    createInstance(AUS_Ci.nsIScriptableUnicodeConverter);
+    converter.charset = "UTF-8";
+    let data = converter.convertToByteArray(exeFile.path.toLowerCase());
+
+    hasher.update(data, data.length);
+    return "Global\\MozillaUpdateMutex-" + hasher.finish(true);
+  }
+
+  /**
+   * Closes a Win32 handle.
+   *
+   * @param  aHandle
+   *         The handle to close.
+   */
+  function closeHandle(aHandle) {
+    let lib = ctypes.open("kernel32.dll");
+    let CloseHandle = lib.declare("CloseHandle",
+                                  ctypes.winapi_abi,
+                                  ctypes.int32_t, /* success */
+                                  ctypes.void_t.ptr); /* handle */
+    CloseHandle(aHandle);
+    lib.close();
+  }
+
+  /**
+   * Creates a mutex.
+   *
+   * @param  aName
+   *         The name for the mutex.
+   * @return The Win32 handle to the mutex.
+   */
+  function createMutex(aName) {
+    const INITIAL_OWN = 1;
+    const ERROR_ALREADY_EXISTS = 0xB7;
+    let lib = ctypes.open("kernel32.dll");
+    let CreateMutexW = lib.declare("CreateMutexW",
+                                   ctypes.winapi_abi,
+                                   ctypes.void_t.ptr, /* return handle */
+                                   ctypes.void_t.ptr, /* security attributes */
+                                   ctypes.int32_t, /* initial owner */
+                                   ctypes.jschar.ptr); /* name */
+
+    let handle = CreateMutexW(null, INITIAL_OWN, aName);
+    lib.close();
+    let alreadyExists = ctypes.winLastError == ERROR_ALREADY_EXISTS;
+    if (handle && !handle.isNull() && alreadyExists) {
+      closeHandle(handle);
+      handle = null;
+    }
+
+    if (handle && handle.isNull()) {
+      handle = null;
+    }
+
+    return handle;
+  }
+}