Bug 1376612 - Relaunch callback app even when the pid is still present. r=mhowell
authorRobert Strong <robert.bugzilla@gmail.com>
Wed, 28 Jun 2017 16:18:09 -0700
changeset 601754 43c421b4a7638cd46e973105b8b36c09ef0a9b7d
parent 601753 37651b6bd99ceaeffa17b2a2e02f6e846fa3a3cf
child 601755 8d1f078804e6b25d51d944071ac23fb818ad6a06
push id66200
push userhchang@mozilla.com
push dateThu, 29 Jun 2017 03:53:43 +0000
reviewersmhowell
bugs1376612, 1375549
milestone56.0a1
Bug 1376612 - Relaunch callback app even when the pid is still present. r=mhowell This reverts the change made in Bug 1375549 where the callback application isn't relaunched when the pid is still present. Since the end result of relaunching when the pid hasn't exited is no worse than not relaunching just go ahead and try to relaunch.
toolkit/mozapps/update/tests/unit_base_updater/marPIDPersistsSuccessComplete_win.js
toolkit/mozapps/update/updater/updater.cpp
--- a/toolkit/mozapps/update/tests/unit_base_updater/marPIDPersistsSuccessComplete_win.js
+++ b/toolkit/mozapps/update/tests/unit_base_updater/marPIDPersistsSuccessComplete_win.js
@@ -37,24 +37,29 @@ function waitForHelperSleepFinished() {
 function runUpdateFinished() {
   waitForHelperExit();
 }
 
 /**
  * Called after the call to waitForHelperExit finishes.
  */
 function waitForHelperExitFinished() {
+  checkPostUpdateAppLog();
+}
+
+/**
+ * Called after the call to checkPostUpdateAppLog finishes.
+ */
+function checkPostUpdateAppLogFinished() {
   standardInit();
   Assert.equal(readStatusState(), STATE_NONE,
                "the status file state" + MSG_SHOULD_EQUAL);
   Assert.ok(!gUpdateManager.activeUpdate,
             "the active update should not be defined");
   Assert.equal(gUpdateManager.updateCount, 1,
                "the update manager updateCount attribute" + MSG_SHOULD_EQUAL);
   Assert.equal(gUpdateManager.getUpdateAt(0).state, STATE_SUCCEEDED,
                "the update state" + MSG_SHOULD_EQUAL);
   checkPostUpdateRunningFile(true);
   checkFilesAfterUpdateSuccess(getApplyDirFile);
   checkUpdateLogContains(ERR_PARENT_PID_PERSISTS);
-  // The callback application is not launched by the updater when the parent pid
-  // doesn't exit.
-  waitForFilesInUse();
+  checkCallbackLog();
 }
--- a/toolkit/mozapps/update/updater/updater.cpp
+++ b/toolkit/mozapps/update/updater/updater.cpp
@@ -2091,18 +2091,17 @@ LaunchWinPostProcess(const WCHAR *instal
 }
 
 #endif
 
 static void
 LaunchCallbackApp(const NS_tchar *workingDir,
                   int argc,
                   NS_tchar **argv,
-                  bool usingService,
-                  NS_tpid pid)
+                  bool usingService)
 {
   putenv(const_cast<char*>("NO_EM_RESTART="));
   putenv(const_cast<char*>("MOZ_LAUNCHED_CHILD=1"));
 
   // Run from the specified working directory (see bug 312360).
   if (NS_tchdir(workingDir) != 0) {
     LOG(("Warning: chdir failed"));
   }
@@ -2110,25 +2109,16 @@ LaunchCallbackApp(const NS_tchar *workin
 #if defined(USE_EXECV)
   execv(argv[0], argv);
 #elif defined(XP_MACOSX)
   LaunchChild(argc, (const char**)argv);
 #elif defined(XP_WIN)
   // Do not allow the callback to run when running an update through the
   // service as session 0.  The unelevated updater.exe will do the launching.
   if (!usingService) {
-    // If there was a pid specified don't run the callback application if the
-    // pid is still present.
-    if (pid > 0) {
-      HANDLE parent = OpenProcess(SYNCHRONIZE, false, (DWORD) pid);
-      if (parent) {
-        CloseHandle(parent);
-        return;
-      }
-    }
     WinLaunchChild(argv[0], argc, argv, nullptr);
   }
 #else
 # warning "Need implementaton of LaunchCallbackApp"
 #endif
 }
 
 static bool
@@ -2652,17 +2642,17 @@ void freeArguments(int argc, char** argv
 int LaunchCallbackAndPostProcessApps(int argc, NS_tchar** argv,
                                      int callbackIndex
 #ifdef XP_WIN
                                      , const WCHAR* elevatedLockFilePath
                                      , HANDLE updateLockFileHandle
 #elif XP_MACOSX
                                      , bool isElevated
 #endif
-                                     , NS_tpid pid)
+                                     )
 {
   if (argc > callbackIndex) {
 #if defined(XP_WIN)
     if (gSucceeded) {
       if (!LaunchWinPostProcess(gInstallDirPath, gPatchDirPath)) {
         fprintf(stderr, "The post update process was not launched");
       }
 
@@ -2683,17 +2673,17 @@ int LaunchCallbackAndPostProcessApps(int
       if (gSucceeded) {
         LaunchMacPostProcess(gInstallDirPath);
       }
 #endif
 
     LaunchCallbackApp(argv[5],
                       argc - callbackIndex,
                       argv + callbackIndex,
-                      sUsingService, pid);
+                      sUsingService);
 #ifdef XP_MACOSX
     } // if (!isElevated)
 #endif /* XP_MACOSX */
   }
   return 0;
 }
 
 int NS_main(int argc, NS_tchar **argv)
@@ -2940,17 +2930,17 @@ int NS_main(int argc, NS_tchar **argv)
     Thread t1;
     if (t1.Run(ServeElevatedUpdateThreadFunc, &threadArgs) == 0) {
       // Show an indeterminate progress bar while an elevated update is in
       // progress.
       ShowProgressUI(true);
     }
     t1.Join();
 
-    LaunchCallbackAndPostProcessApps(argc, argv, callbackIndex, false, pid);
+    LaunchCallbackAndPostProcessApps(argc, argv, callbackIndex, false);
     return gSucceeded ? 0 : 1;
   }
 #endif
 
   LogInit(gPatchDirPath, NS_T("update.log"));
 
   if (!WriteStatusFile("applying")) {
     LOG(("failed setting status to 'applying'"));
@@ -3311,17 +3301,17 @@ int NS_main(int argc, NS_tchar **argv)
           CloseHandle(sinfo.hProcess);
         } else {
           WriteStatusFile(ELEVATION_CANCELED);
         }
       }
 
       if (argc > callbackIndex) {
         LaunchCallbackApp(argv[5], argc - callbackIndex,
-                          argv + callbackIndex, sUsingService, pid);
+                          argv + callbackIndex, sUsingService);
       }
 
       CloseHandle(elevatedFileHandle);
 
       if (!useService && !noServiceFallback &&
           INVALID_HANDLE_VALUE == updateLockFileHandle) {
         // We didn't use the service and we did run the elevated updater.exe.
         // The elevated updater.exe is responsible for writing out the
@@ -3396,17 +3386,17 @@ int NS_main(int argc, NS_tchar **argv)
   if (!GetLongPathNameW(gWorkingDirPath, applyDirLongPath,
                         sizeof(applyDirLongPath) / sizeof(applyDirLongPath[0]))) {
     LOG(("NS_main: unable to find apply to dir: " LOG_S, gWorkingDirPath));
     LogFinish();
     WriteStatusFile(WRITE_ERROR_APPLY_DIR_PATH);
     EXIT_WHEN_ELEVATED(elevatedLockFilePath, updateLockFileHandle, 1);
     if (argc > callbackIndex) {
       LaunchCallbackApp(argv[5], argc - callbackIndex,
-                        argv + callbackIndex, sUsingService, pid);
+                        argv + callbackIndex, sUsingService);
     }
     return 1;
   }
 
   HANDLE callbackFile = INVALID_HANDLE_VALUE;
   if (argc > callbackIndex) {
     // If the callback executable is specified it must exist for a successful
     // update.  It is important we null out the whole buffer here because later
@@ -3451,17 +3441,17 @@ int NS_main(int argc, NS_tchar **argv)
       LOG(("NS_main: unable to find callback file: " LOG_S, targetPath));
       LogFinish();
       WriteStatusFile(WRITE_ERROR_CALLBACK_PATH);
       EXIT_WHEN_ELEVATED(elevatedLockFilePath, updateLockFileHandle, 1);
       if (argc > callbackIndex) {
         LaunchCallbackApp(argv[5],
                           argc - callbackIndex,
                           argv + callbackIndex,
-                          sUsingService, pid);
+                          sUsingService);
       }
       return 1;
     }
 
     // Doing this is only necessary when we're actually applying a patch.
     if (!sReplaceRequest) {
       int len = NS_tstrlen(applyDirLongPath);
       NS_tchar *s = callbackLongPath;
@@ -3515,17 +3505,17 @@ int NS_main(int argc, NS_tchar **argv)
         } else {
           WriteStatusFile(WRITE_ERROR_CALLBACK_APP);
         }
 
         EXIT_WHEN_ELEVATED(elevatedLockFilePath, updateLockFileHandle, 1);
         LaunchCallbackApp(argv[callbackIndex],
                           argc - callbackIndex,
                           argv + callbackIndex,
-                          sUsingService, pid);
+                          sUsingService);
         return 1;
       }
 
       // Since the process may be signaled as exited by WaitForSingleObject before
       // the release of the executable image try to lock the main executable file
       // multiple times before giving up.  If we end up giving up, we won't
       // fail the update.
       const int max_retries = 10;
@@ -3564,17 +3554,17 @@ int NS_main(int argc, NS_tchar **argv)
             WriteStatusFile(WRITE_ERROR_CALLBACK_APP);
           }
 
           NS_tremove(gCallbackBackupPath);
           EXIT_WHEN_ELEVATED(elevatedLockFilePath, updateLockFileHandle, 1);
           LaunchCallbackApp(argv[5],
                             argc - callbackIndex,
                             argv + callbackIndex,
-                            sUsingService, pid);
+                            sUsingService);
           return 1;
         }
         LOG(("NS_main: callback app file in use, continuing without " \
              "exclusive access for executable file: " LOG_S,
              argv[callbackIndex]));
       }
     }
   }
@@ -3703,17 +3693,17 @@ int NS_main(int argc, NS_tchar **argv)
 
   int retVal = LaunchCallbackAndPostProcessApps(argc, argv, callbackIndex
 #ifdef XP_WIN
                                                 , elevatedLockFilePath
                                                 , updateLockFileHandle
 #elif XP_MACOSX
                                                 , isElevated
 #endif
-                                                , pid);
+                                                );
 
   return retVal ? retVal : (gSucceeded ? 0 : 1);
 }
 
 class ActionList
 {
 public:
   ActionList() : mFirst(nullptr), mLast(nullptr), mCount(0) { }