Bug 711692 - Various fixes for intermittent failures. r=rstrong.
authorBrian R. Bondy <netzen@gmail.com>
Wed, 28 Dec 2011 21:08:37 -0500
changeset 83795 10894668e37f17be5b8bee6fa33fa24eb0b534c0
parent 83794 e5481fffac9eed0219069eba9d109f5cfcfbe283
child 83800 4795500b7c1d62f7659c9cd2e0c26e8b7a705bd4
push id21792
push userbbondy@mozilla.com
push dateThu, 05 Jan 2012 04:20:59 +0000
treeherdermozilla-central@10894668e37f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrstrong
bugs711692
milestone12.0a1
first release with
nightly linux32
10894668e37f / 12.0a1 / 20120105031049 / files
nightly linux64
10894668e37f / 12.0a1 / 20120105031049 / files
nightly mac
10894668e37f / 12.0a1 / 20120105031049 / files
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
Bug 711692 - Various fixes for intermittent failures. r=rstrong.
browser/installer/windows/nsis/maintenanceservice_installer.nsi
toolkit/components/maintenanceservice/maintenanceservice.cpp
toolkit/components/maintenanceservice/maintenanceservice.h
toolkit/components/maintenanceservice/serviceinstall.cpp
toolkit/components/maintenanceservice/workmonitor.cpp
toolkit/mozapps/update/common/uachelper.cpp
toolkit/mozapps/update/common/updatehelper.cpp
toolkit/mozapps/update/common/updatehelper.h
toolkit/mozapps/update/nsUpdateService.js
toolkit/mozapps/update/test/TestAUSHelper.cpp
toolkit/mozapps/update/test/unit/head_update.js.in
toolkit/mozapps/update/test_svc/unit/test_0000_bootstrap_svc.js
toolkit/mozapps/update/updater/updater.cpp
toolkit/xre/nsAppRunner.cpp
toolkit/xre/nsWindowsRestart.cpp
--- a/browser/installer/windows/nsis/maintenanceservice_installer.nsi
+++ b/browser/installer/windows/nsis/maintenanceservice_installer.nsi
@@ -30,19 +30,16 @@
 # use your version of this file under the terms of the MPL, indicate your
 # decision by deleting the provisions above and replace them with the notice
 # and other provisions required by the GPL or the LGPL. If you do not delete
 # the provisions above, a recipient may use your version of this file under
 # the terms of any one of the MPL, the GPL or the LGPL.
 #
 # ***** END LICENSE BLOCK *****
 
-# Required Plugins:
-# ServicesHelper Mozilla specific plugin that is located in /other-licenses/nsis
-
 ; Set verbosity to 3 (e.g. no script) to lessen the noise in the build logs
 !verbose 3
 
 ; 7-Zip provides better compression than the lzma from NSIS so we add the files
 ; uncompressed and use 7-Zip to create a SFX archive of it
 SetDatablockOptimize on
 SetCompress off
 CRCCheck on
@@ -165,28 +162,22 @@ Function un.onInit
 FunctionEnd
 
 Section "MaintenanceService"
   AllowSkipFiles off
 
   CreateDirectory $INSTDIR
   SetOutPath $INSTDIR
 
-  ; If the service already exists, then stop it if it is running.
-  ServicesHelper::IsInstalled "MozillaMaintenance"
-  Pop $R9
-  ${If} $R9 == 1
-    ; Stop the maintenance service so we can overwrite any
-    ; binaries that it uses.
-    ServicesHelper::Stop "MozillaMaintenance"
-  ${EndIf}
-
-  ; If we don't have maintenanceservice.exe already installed
-  ; then keep that name.  If we do use maintenanceservice_tmp.exe
-  ; which will auto install itself when you call it with the install parameter.
+  ; If the service already exists, then it will be stopped when upgrading it
+  ; via the maintenanceservice_tmp.exe command executed below.
+  ; The maintenanceservice_tmp.exe command will rename the file to
+  ; maintenanceservice.exe if maintenanceservice_tmp.exe is newer.
+  ; If the service does not exist yet, we install it and drop the file on
+  ; disk as maintenanceservice.exe directly.
   StrCpy $TempMaintServiceName "maintenanceservice.exe"
   IfFileExists "$INSTDIR\maintenanceservice.exe" 0 skipAlreadyExists
     StrCpy $TempMaintServiceName "maintenanceservice_tmp.exe"
   skipAlreadyExists:
 
   ; We always write out a copy and then decide whether to install it or 
   ; not via calling its 'install' cmdline which works by version comparison.
   CopyFiles "$EXEDIR\maintenanceservice.exe" "$INSTDIR\$TempMaintServiceName"
--- a/toolkit/components/maintenanceservice/maintenanceservice.cpp
+++ b/toolkit/components/maintenanceservice/maintenanceservice.cpp
@@ -45,18 +45,19 @@
 #include "maintenanceservice.h"
 #include "servicebase.h"
 #include "workmonitor.h"
 #include "uachelper.h"
 #include "updatehelper.h"
 
 SERVICE_STATUS gSvcStatus = { 0 }; 
 SERVICE_STATUS_HANDLE gSvcStatusHandle = NULL; 
-HANDLE ghSvcStopEvent = NULL;
-BOOL gServiceStopping = FALSE;
+HANDLE gWorkDoneEvent = NULL;
+HANDLE gThread = NULL;
+bool gServiceControlStopping = false;
 
 // logs are pretty small ~10 lines, so 5 seems reasonable.
 #define LOGS_TO_KEEP 5
 
 BOOL GetLogDirectoryPath(WCHAR *path);
 
 int 
 wmain(int argc, WCHAR **argv)
@@ -124,51 +125,37 @@ wmain(int argc, WCHAR **argv)
       return 1;
     }
     LOG(("The service was uninstalled successfully\n"));
     LogFinish();
     return 0;
   }
 
   SERVICE_TABLE_ENTRYW DispatchTable[] = { 
-    { SVC_NAME, (LPSERVICE_MAIN_FUNCTION) SvcMain }, 
+    { SVC_NAME, (LPSERVICE_MAIN_FUNCTIONW) SvcMain }, 
     { NULL, NULL } 
   }; 
 
   // This call returns when the service has stopped. 
   // The process should simply terminate when the call returns.
-  if (!StartServiceCtrlDispatcher(DispatchTable)) {
+  if (!StartServiceCtrlDispatcherW(DispatchTable)) {
     LOG(("StartServiceCtrlDispatcher failed (%d)\n", GetLastError()));
   }
 
   return 0;
 }
 
 /**
- * Wrapper callback for the monitoring thread.
- *
- * @param param Unused thread callback parameter
- */
-DWORD
-WINAPI StartMaintenanceServiceThread(LPVOID param) 
-{
-  ThreadData *threadData = reinterpret_cast<ThreadData*>(param);
-  ExecuteServiceCommand(threadData->argc, threadData->argv);
-  delete threadData;
-  return 0;
-}
-
-/**
  * Obtains the base path where logs should be stored
  *
  * @param  path The out buffer for the backup log path of size MAX_PATH + 1
  * @return TRUE if successful.
  */
 BOOL
-GetLogDirectoryPath(WCHAR *path) 
+GetLogDirectoryPath(WCHAR *path)
 {
   HRESULT hr = SHGetFolderPathW(NULL, CSIDL_COMMON_APPDATA, NULL, 
     SHGFP_TYPE_CURRENT, path);
   if (FAILED(hr)) {
     return FALSE;
   }
 
   if (!PathAppendSafe(path, L"Mozilla")) {
@@ -213,101 +200,129 @@ GetBackupLogPath(LPWSTR path, LPCWSTR ba
  *   updater1.log -> updater2.log
  *   updater.log -> updater1.log
  * Which clears room for a new updater.log in the basePath directory
  * 
  * @param basePath      The base directory path where log files are stored
  * @param numLogsToKeep The number of logs to keep
  */
 void
-BackupOldLogs(LPCWSTR basePath, int numLogsToKeep) 
+BackupOldLogs(LPCWSTR basePath, int numLogsToKeep)
 {
   WCHAR oldPath[MAX_PATH + 1];
   WCHAR newPath[MAX_PATH + 1];
   for (int i = numLogsToKeep; i >= 1; i--) {
     if (!GetBackupLogPath(oldPath, basePath, i -1)) {
       continue;
     }
 
     if (!GetBackupLogPath(newPath, basePath, i)) {
       continue;
     }
 
-    if (!MoveFileEx(oldPath, newPath, MOVEFILE_REPLACE_EXISTING)) {
+    if (!MoveFileExW(oldPath, newPath, MOVEFILE_REPLACE_EXISTING)) {
       continue;
     }
   }
 }
 
 /**
+ * Ensures the service is shutdown once all work is complete.
+ * There is an issue on XP SP2 and below where the service can hang
+ * in a stop pending state even though the SCM is notified of a stopped
+ * state.  Control *should* be returned to StartServiceCtrlDispatcher from the 
+ * call to SetServiceStatus on a stopped state in the wmain thread.  
+ * Sometimes this is not the case though. This thread will terminate the process
+ * if it has been 5 seconds after all work is done and the process is still not
+ * terminated.  This thread is only started once a stopped state was sent to the
+ * SCM. The stop pending hang can be reproduced intermittently even if you set 
+ * a stopped state dirctly and never set a stop pending state.  It is safe to 
+ * forcefully terminate the process ourselves since all work is done once we 
+ * start this thread.
+*/
+DWORD WINAPI
+EnsureProcessTerminatedThread(LPVOID)
+{
+  Sleep(5000);
+  exit(0);
+  return 0;
+}
+
+void
+StartTerminationThread()
+{
+  // If the process does not self terminate like it should, this thread 
+  // will terminate the process after 5 seconds.
+  HANDLE thread = CreateThread(NULL, 0, EnsureProcessTerminatedThread, 
+                               NULL, 0, NULL);
+  if (thread) {
+    CloseHandle(thread);
+  }
+}
+
+/**
  * Main entry point when running as a service.
  */
-void WINAPI 
-SvcMain(DWORD dwArgc, LPWSTR *lpszArgv)
+void WINAPI
+SvcMain(DWORD argc, LPWSTR *argv)
 {
   // Setup logging, and backup the old logs
   WCHAR updatePath[MAX_PATH + 1];
   if (GetLogDirectoryPath(updatePath)) {
     BackupOldLogs(updatePath, LOGS_TO_KEEP);
     LogInit(updatePath, L"maintenanceservice.log");
   }
 
   // Disable every privilege we don't need. Processes started using
   // CreateProcess will use the same token as this process.
   UACHelper::DisablePrivileges(NULL);
 
   // Register the handler function for the service
   gSvcStatusHandle = RegisterServiceCtrlHandlerW(SVC_NAME, SvcCtrlHandler);
   if (!gSvcStatusHandle) {
     LOG(("RegisterServiceCtrlHandler failed (%d)\n", GetLastError()));
-    return; 
+    ExecuteServiceCommand(argc, argv);  
+    LogFinish();
+    exit(1);
   } 
 
-  // These SERVICE_STATUS members remain as set here
+  // These values will be re-used later in calls involving gSvcStatus
   gSvcStatus.dwServiceType = SERVICE_WIN32_OWN_PROCESS;
   gSvcStatus.dwServiceSpecificExitCode = 0;
 
   // Report initial status to the SCM
   ReportSvcStatus(SERVICE_START_PENDING, NO_ERROR, 3000);
 
-  // Perform service-specific initialization and work.
-  SvcInit(dwArgc, lpszArgv);
-}
-
-/**
- * Service initialization.
- */
-void
-SvcInit(DWORD argc, LPWSTR *argv)
-{
-  // Create an event. The control handler function, SvcCtrlHandler,
-  // signals this event when it receives the stop control code.
-  ghSvcStopEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
-  if (NULL == ghSvcStopEvent) {
+  // This event will be used to tell the SvcCtrlHandler when the work is
+  // done for when a stop comamnd is manually issued.
+  gWorkDoneEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
+  if (!gWorkDoneEvent) {
     ReportSvcStatus(SERVICE_STOPPED, 1, 0);
+    StartTerminationThread();
     return;
   }
 
-  ThreadData *threadData = new ThreadData();
-  threadData->argc = argc;
-  threadData->argv = argv;
-
-  DWORD threadID;
-  HANDLE thread = CreateThread(NULL, 0, StartMaintenanceServiceThread, 
-                               threadData, 0, &threadID);
-
-  // Report running status when initialization is complete.
+  // Initialization complete and we're about to start working on
+  // the actual command.  Report the service state as running to the SCM.
   ReportSvcStatus(SERVICE_RUNNING, NO_ERROR, 0);
 
-  // Perform work until service stops.
-  for(;;) {
-    // Check whether to stop the service.
-    WaitForSingleObject(ghSvcStopEvent, INFINITE);
+  // The service command was executed, stop logging and set an event
+  // to indicate the work is done in case someone is waiting on a
+  // service stop operation.
+  ExecuteServiceCommand(argc, argv);  
+  LogFinish();
+
+  SetEvent(gWorkDoneEvent);
+
+  // If we aren't already in a stopping state then tell the SCM we're stopped
+  // now.  If we are already in a stopping state then the SERVICE_STOPPED state
+  // will be set by the SvcCtrlHandler.
+  if (!gServiceControlStopping) {
     ReportSvcStatus(SERVICE_STOPPED, NO_ERROR, 0);
-    return;
+    StartTerminationThread();
   }
 }
 
 /**
  * Sets the current service status and reports it to the SCM.
  *  
  * @param currentState  The current state (see SERVICE_STATUS)
  * @param exitCode      The system error code
@@ -315,52 +330,86 @@ SvcInit(DWORD argc, LPWSTR *argv)
  */
 void
 ReportSvcStatus(DWORD currentState, 
                 DWORD exitCode, 
                 DWORD waitHint)
 {
   static DWORD dwCheckPoint = 1;
 
-  // Fill in the SERVICE_STATUS structure.
   gSvcStatus.dwCurrentState = currentState;
   gSvcStatus.dwWin32ExitCode = exitCode;
   gSvcStatus.dwWaitHint = waitHint;
 
-  if (SERVICE_START_PENDING == currentState) {
+  if (SERVICE_START_PENDING == currentState || 
+      SERVICE_STOP_PENDING == currentState) {
     gSvcStatus.dwControlsAccepted = 0;
   } else {
-    gSvcStatus.dwControlsAccepted = SERVICE_ACCEPT_STOP;
+    gSvcStatus.dwControlsAccepted = SERVICE_ACCEPT_STOP | 
+                                    SERVICE_ACCEPT_SHUTDOWN;
   }
 
   if ((SERVICE_RUNNING == currentState) ||
       (SERVICE_STOPPED == currentState)) {
     gSvcStatus.dwCheckPoint = 0;
   } else {
     gSvcStatus.dwCheckPoint = dwCheckPoint++;
   }
 
   // Report the status of the service to the SCM.
   SetServiceStatus(gSvcStatusHandle, &gSvcStatus);
 }
 
 /**
+ * Since the SvcCtrlHandler should only spend at most 30 seconds before 
+ * returning, this function does the service stop work for the SvcCtrlHandler. 
+*/
+DWORD WINAPI
+StopServiceAndWaitForCommandThread(LPVOID)
+{
+  do {
+    ReportSvcStatus(SERVICE_STOP_PENDING, NO_ERROR, 1000);
+  } while(WaitForSingleObject(gWorkDoneEvent, 100) == WAIT_TIMEOUT);
+  CloseHandle(gWorkDoneEvent);
+  gWorkDoneEvent = NULL;
+  ReportSvcStatus(SERVICE_STOPPED, NO_ERROR, 0);
+  StartTerminationThread();
+  return 0;
+}
+
+/**
  * Called by SCM whenever a control code is sent to the service
  * using the ControlService function.
  */
 void WINAPI
 SvcCtrlHandler(DWORD dwCtrl)
 {
+  // After a SERVICE_CONTROL_STOP there should be no more commands sent to
+  // the SvcCtrlHandler.
+  if (gServiceControlStopping) {
+    return;
+  }
+
   // Handle the requested control code. 
   switch(dwCtrl) {
-  case SERVICE_CONTROL_STOP: 
-    ReportSvcStatus(SERVICE_STOP_PENDING, NO_ERROR, 0);
-    // Signal the service to stop.
-    SetEvent(ghSvcStopEvent);
-    ReportSvcStatus(gSvcStatus.dwCurrentState, NO_ERROR, 0);
-    LogFinish();
+  case SERVICE_CONTROL_SHUTDOWN:
+  case SERVICE_CONTROL_STOP: {
+      gServiceControlStopping = true;
+      ReportSvcStatus(SERVICE_STOP_PENDING, NO_ERROR, 1000);
+
+      // The SvcCtrlHandler thread should not spend more than 30 seconds in 
+      // shutdown so we spawn a new thread for stopping the service 
+      HANDLE thread = CreateThread(NULL, 0, StopServiceAndWaitForCommandThread, 
+                                   NULL, 0, NULL);
+      if (thread) {
+        CloseHandle(thread);
+      } else {
+        // Couldn't start the thread so just call the stop ourselves.
+        // If it happens to take longer than 30 seconds the caller will
+        // get an error.
+        StopServiceAndWaitForCommandThread(NULL);
+      }
+    }
     break;
-  case SERVICE_CONTROL_INTERROGATE: 
-    break; 
-  default: 
+  default:
     break;
   }
 }
--- a/toolkit/components/maintenanceservice/maintenanceservice.h
+++ b/toolkit/components/maintenanceservice/maintenanceservice.h
@@ -36,12 +36,8 @@
  * ***** END LICENSE BLOCK ***** */
 
 void WINAPI SvcMain(DWORD dwArgc, LPWSTR *lpszArgv);
 void SvcInit(DWORD dwArgc, LPWSTR *lpszArgv);
 void WINAPI SvcCtrlHandler(DWORD dwCtrl);
 void ReportSvcStatus(DWORD dwCurrentState, 
                      DWORD dwWin32ExitCode, 
                      DWORD dwWaitHint);
-struct ThreadData {
-  LPWSTR *argv;
-  DWORD argc;
-};
--- a/toolkit/components/maintenanceservice/serviceinstall.cpp
+++ b/toolkit/components/maintenanceservice/serviceinstall.cpp
@@ -184,44 +184,45 @@ SvcInstall(SvcInstallAction action)
         (existingA == newA && existingB == newB && 
          existingC < newC) ||
         (existingA == newA && existingB == newB && 
          existingC == newC && existingD < newD)) {
       if (!StopService()) {
         return FALSE;
       }
 
-      if (!DeleteFile(serviceConfig.lpBinaryPathName)) {
-        LOG(("Could not delete old service binary file.  (%d)\n", GetLastError()));
+      if (!DeleteFileW(serviceConfig.lpBinaryPathName)) {
+        LOG(("Could not delete old service binary file %ls.  (%d)\n", 
+             serviceConfig.lpBinaryPathName, GetLastError()));
         return FALSE;
       }
 
-      if (!CopyFile(newServiceBinaryPath, 
+      if (!CopyFileW(newServiceBinaryPath, 
                     serviceConfig.lpBinaryPathName, FALSE)) {
         LOG(("Could not overwrite old service binary file. "
              "This should never happen, but if it does the next upgrade will fix"
              " it, the service is not a critical component that needs to be "
              " installed for upgrades to work. (%d)\n", 
              GetLastError()));
         return FALSE;
       }
 
       // We made a copy of ourselves to the existing location.
       // The tmp file (the process of which we are executing right now) will be
       // left over.  Attempt to delete the file on the next reboot.
-      MoveFileEx(newServiceBinaryPath, NULL, MOVEFILE_DELAY_UNTIL_REBOOT);
+      MoveFileExW(newServiceBinaryPath, NULL, MOVEFILE_DELAY_UNTIL_REBOOT);
       
       // Setup the new module path
       wcsncpy(newServiceBinaryPath, serviceConfig.lpBinaryPathName, MAX_PATH);
       // Fall through so we replace the service
     } else {
       // We don't need to copy ourselves to the existing location.
       // The tmp file (the process of which we are executing right now) will be
       // left over.  Attempt to delete the file on the next reboot.
-      MoveFileEx(newServiceBinaryPath, NULL, MOVEFILE_DELAY_UNTIL_REBOOT);
+      MoveFileExW(newServiceBinaryPath, NULL, MOVEFILE_DELAY_UNTIL_REBOOT);
       
       return TRUE; // nothing to do, we already have a newer service installed
     }
   } else if (UpgradeSvc == action) {
     // The service does not exist and we are upgrading, so don't install it
     return TRUE;
   }
 
@@ -269,20 +270,34 @@ StopService()
   // Open the service
   nsAutoServiceHandle schService(OpenServiceW(schSCManager, SVC_NAME, 
                                               SERVICE_ALL_ACCESS));
   if (!schService) {
     LOG(("Could not open service.  (%d)\n", GetLastError()));
     return FALSE;
   } 
 
-  // Stop logging before stopping the service.
-  LogFinish();
+  LOG(("Sending stop request...\n"));
+  SERVICE_STATUS status;
+  if (!ControlService(schService, SERVICE_CONTROL_STOP, &status)) {
+    LOG(("Error sending stop request: %d\n", GetLastError()));
+  }
+
+  schSCManager.reset();
+  schService.reset();
 
-  return WaitForServiceStop(SVC_NAME, 60); 
+  LOG(("Waiting for service stop...\n"));
+  DWORD lastState = WaitForServiceStop(SVC_NAME, 30);
+
+  // The service can be in a stopped state but the exe still in use
+  // so make sure the process is really gone before proceeding
+  WaitForProcessExit(L"maintenanceservice.exe", 30);
+  LOG(("Done waiting for service stop, last service state: %d\n", lastState));
+
+  return lastState == SERVICE_STOPPED;
 }
 
 /**
  * Uninstalls the Maintenance service.
  *
  * @return TRUE if successful.
  */
 BOOL
--- a/toolkit/components/maintenanceservice/workmonitor.cpp
+++ b/toolkit/components/maintenanceservice/workmonitor.cpp
@@ -52,34 +52,71 @@
 
 #include "workmonitor.h"
 #include "serviceinstall.h"
 #include "servicebase.h"
 #include "registrycertificates.h"
 #include "uachelper.h"
 #include "updatehelper.h"
 
-extern HANDLE ghSvcStopEvent;
-
 // Wait 15 minutes for an update operation to run at most.
 // Updates usually take less than a minute so this seems like a 
 // significantly large and safe amount of time to wait.
 static const int TIME_TO_WAIT_ON_UPDATER = 15 * 60 * 1000;
 PRUnichar* MakeCommandLine(int argc, PRUnichar **argv);
 BOOL WriteStatusFailure(LPCWSTR updateDirPath, int errorCode);
 BOOL PathGetSiblingFilePath(LPWSTR destinationBuffer,  LPCWSTR siblingFilePath, 
                             LPCWSTR newFileName);
 
 // The error codes start from 16000 since Windows system error 
 // codes only go up to 15999
 const int SERVICE_UPDATER_COULD_NOT_BE_STARTED = 16000;
 const int SERVICE_NOT_ENOUGH_COMMAND_LINE_ARGS = 16001;
 const int SERVICE_UPDATER_SIGN_ERROR = 16002;
 const int SERVICE_UPDATER_COMPARE_ERROR = 16003;
 const int SERVICE_UPDATER_IDENTITY_ERROR = 16004;
+const int SERVICE_STILL_APPLYING_ON_SUCCESS = 16005;
+const int SERVICE_STILL_APPLYING_ON_FAILURE = 16006;
+
+/* 
+ * Read the update.status file and sets isApplying to true if
+ * the status is set to applying
+ *
+ * @param  updateDirPath The directory where update.status is stored
+ * @param  isApplying Out parameter for specifying if the status
+ *         is set to applying or not.
+ * @return TRUE if the information was filled.
+*/
+static BOOL
+IsStatusApplying(LPCWSTR updateDirPath, BOOL &isApplying)
+{
+  isApplying = FALSE;
+  WCHAR updateStatusFilePath[MAX_PATH + 1];
+  wcscpy(updateStatusFilePath, updateDirPath);
+  if (!PathAppendSafe(updateStatusFilePath, L"update.status")) {
+    return FALSE;
+  }
+
+  nsAutoHandle statusFile(CreateFileW(updateStatusFilePath, GENERIC_READ,
+                                      FILE_SHARE_READ | 
+                                      FILE_SHARE_WRITE | 
+                                      FILE_SHARE_DELETE,
+                                      NULL, OPEN_EXISTING, 0, NULL));
+  char buf[32] = { 0 };
+  DWORD read;
+  if (!ReadFile(statusFile, buf, sizeof(buf), &read, NULL)) {
+    return FALSE;
+  }
+
+  const char kApplying[] = "applying";
+  isApplying = strncmp(buf, kApplying, 
+                       sizeof(kApplying) - 1) == 0;
+  return TRUE;
+}
+
 
 /**
  * Runs an update process as the service using the SYSTEM account.
  *
  * @param  argc           The number of arguments in argv
  * @param  argv           The arguments normally passed to updater.exe
  *                        argv[0] must be the path to updater.exe
  * @param  processStarted Set to TRUE if the process was started.
@@ -119,35 +156,32 @@ StartUpdateProcess(int argc,
   // that if there were any errors the next updater.exe will run correctly.
   WCHAR updaterINI[MAX_PATH + 1];
   WCHAR updaterINITemp[MAX_PATH + 1];
   BOOL selfHandlePostUpdate = FALSE;
   // We use the updater.ini from the same directory as the updater.exe
   // because of background updates.
   if (PathGetSiblingFilePath(updaterINI, argv[0], L"updater.ini") &&
       PathGetSiblingFilePath(updaterINITemp, argv[0], L"updater.tmp")) {
-    selfHandlePostUpdate = MoveFileEx(updaterINI, updaterINITemp, 
-                                      MOVEFILE_REPLACE_EXISTING);
+    selfHandlePostUpdate = MoveFileExW(updaterINI, updaterINITemp, 
+                                       MOVEFILE_REPLACE_EXISTING);
   }
 
   // Add an env var for MOZ_USING_SERVICE so the updater.exe can
   // do anything special that it needs to do for service updates.
-  // Search in updater.cpp for more info on MOZ_USING_SERVICE
-  // for more info.
-  WCHAR envVarString[32];
-  wsprintf(envVarString, L"MOZ_USING_SERVICE=1"); 
-  _wputenv(envVarString);
-
-  // Empty value on _wputenv is how you remove an env variable in Windows
+  // Search in updater.cpp for more info on MOZ_USING_SERVICE.
+  putenv(const_cast<char*>("MOZ_USING_SERVICE=1"));
+  LOG(("Starting service with cmdline: %ls\n", cmdLine));
   processStarted = CreateProcessW(argv[0], cmdLine, 
                                   NULL, NULL, FALSE, 
                                   CREATE_DEFAULT_ERROR_MODE, 
                                   NULL, 
                                   NULL, &si, &pi);
-  _wputenv(L"MOZ_USING_SERVICE=");
+  // Empty value on putenv is how you remove an env variable in Windows
+  putenv(const_cast<char*>("MOZ_USING_SERVICE="));
   
   BOOL updateWasSuccessful = FALSE;
   if (processStarted) {
     // Wait for the updater process to finish
     LOG(("Process was started... waiting on result.\n")); 
     DWORD waitRes = WaitForSingleObject(pi.hProcess, TIME_TO_WAIT_ON_UPDATER);
     if (WAIT_TIMEOUT == waitRes) {
       // We waited a long period of time for updater.exe and it never finished
@@ -161,28 +195,53 @@ StartUpdateProcess(int argc,
         // updater returns 0 if successful.
         updateWasSuccessful = (returnCode == 0);
       } else {
         LOG(("Process finished but could not obtain return code.\n")); 
       }
     }
     CloseHandle(pi.hProcess);
     CloseHandle(pi.hThread);
+
+    // Check just in case updater.exe didn't change the status from
+    // applying.  If this is the case we report an error.
+    BOOL isApplying = FALSE;
+    if (IsStatusApplying(argv[1], isApplying) && isApplying) {
+      if (updateWasSuccessful) {
+        LOG(("update.status is still applying even know update "
+             " was successful.\n"));
+        if (!WriteStatusFailure(argv[1], 
+                                SERVICE_STILL_APPLYING_ON_SUCCESS)) {
+          LOG(("Could not write update.status still applying on"
+               " success error.\n"));
+        }
+        // Since we still had applying we know updater.exe didn't do its
+        // job correctly.
+        updateWasSuccessful = FALSE;
+      } else {
+        LOG(("update.status is still applying and update was not successful.\n"));
+        if (!WriteStatusFailure(argv[1], 
+                                SERVICE_STILL_APPLYING_ON_FAILURE)) {
+          LOG(("Could not write update.status still applying on"
+               " success error.\n"));
+        }
+      }
+    }
   } else {
     DWORD lastError = GetLastError();
     LOG(("Could not create process as current user, "
          "updaterPath: %ls; cmdLine: %l.  (%d)\n", 
          argv[0], cmdLine, lastError));
   }
 
   // Now that we're done with the update, restore back the updater.ini file
   // We use it ourselves, and also we want it back in case we had any type 
   // of error so that the normal update process can use it.
   if (selfHandlePostUpdate) {
-    MoveFileEx(updaterINITemp, updaterINI, MOVEFILE_REPLACE_EXISTING);
+    MoveFileExW(updaterINITemp, updaterINI, MOVEFILE_REPLACE_EXISTING);
 
     // Only run the PostUpdate if the update was successful
     if (updateWasSuccessful && argc > 2) {
       LPCWSTR installationDir = argv[2];
       LPCWSTR updateInfoDir = argv[1];
 
       // Launch the PostProcess with admin access in session 0.  This is
       // actually launching the post update process but it takes in the 
@@ -206,17 +265,17 @@ StartUpdateProcess(int argc,
  * Processes a software update command
  *
  * @param  argc           The number of arguments in argv
  * @param  argv           The arguments normally passed to updater.exe
  *                        argv[0] must be the path to updater.exe
  * @return TRUE if the update was successful.
  */
 BOOL
-ProessSoftwareUpdateCommand(DWORD argc, LPWSTR *argv)
+ProcessSoftwareUpdateCommand(DWORD argc, LPWSTR *argv)
 {
   BOOL result = TRUE;
   if (argc < 3) {
     LOG(("Not enough command line parameters specified. "
          "Updating update.status.\n"));
 
     // We can only update update.status if argv[1] exists.  argv[1] is
     // the directory where the update.status file exists.
@@ -357,21 +416,20 @@ ProessSoftwareUpdateCommand(DWORD argc, 
  * @param argc The number of arguments in argv
  * @param argv The service command line arguments, argv[0] and argv[1]
  *             and automatically included by Windows.  argv[2] is the
  *             service command.
  *             
  * @return FALSE if there was an error executing the service command.
  */
 BOOL
-ExecuteServiceCommand(int argc, LPWSTR *argv) 
+ExecuteServiceCommand(int argc, LPWSTR *argv)
 {
   if (argc < 3) {
     LOG(("Not enough command line arguments to execute a service command\n"));
-    SetEvent(ghSvcStopEvent);
     return FALSE;
   }
 
   // The tests work by making sure the log has changed, so we put a 
   // unique ID in the log.
   RPC_WSTR guidString = RPC_WSTR(L"");
   GUID guid;
   HRESULT hr = CoCreateGuid(&guid);
@@ -379,23 +437,22 @@ ExecuteServiceCommand(int argc, LPWSTR *
     UuidToString(&guid, &guidString);
   }
   LOG(("Executing service command %ls, ID: %ls\n", 
        argv[2], reinterpret_cast<LPCWSTR>(guidString)));
   RpcStringFree(&guidString);
 
   BOOL result = FALSE;
   if (!lstrcmpi(argv[2], L"software-update")) {
-    result = ProessSoftwareUpdateCommand(argc - 3, argv + 3);
+    result = ProcessSoftwareUpdateCommand(argc - 3, argv + 3);
     // We might not reach here if the service install succeeded
     // because the service self updates itself and the service
     // installer will stop the service.
     LOG(("Service command %ls complete.\n", argv[2]));
   } else {
     LOG(("Service command not recognized: %ls.\n", argv[2]));
     // result is already set to FALSE
   }
 
   LOG(("service command %ls complete with result: %ls.\n", 
        argv[1], (result ? L"Success" : L"Failure")));
-  SetEvent(ghSvcStopEvent);
   return TRUE;
 }
--- a/toolkit/mozapps/update/common/uachelper.cpp
+++ b/toolkit/mozapps/update/common/uachelper.cpp
@@ -85,17 +85,17 @@ LPCTSTR UACHelper::PrivsToDisable[] = {
 };
 
 /**
  * Determines if the OS is vista or later
  *
  * @return TRUE if the OS is vista or later.
  */
 BOOL
-UACHelper::IsVistaOrLater() 
+UACHelper::IsVistaOrLater()
 {
   // Check if we are running Vista or later.
   OSVERSIONINFO osInfo;
   osInfo.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
   return GetVersionEx(&osInfo) && osInfo.dwMajorVersion >= 6;
 }
 
 /**
@@ -122,17 +122,17 @@ UACHelper::OpenUserToken(DWORD sessionID
 /**
  * Opens a linked token for the specified token.
  *
  * @param  token The token to get the linked token from
  * @return A linked token or NULL if one does not exist.
  *         Caller should close the handle.
  */
 HANDLE
-UACHelper::OpenLinkedToken(HANDLE token) 
+UACHelper::OpenLinkedToken(HANDLE token)
 {
   // Magic below...
   // UAC creates 2 tokens.  One is the restricted token which we have.
   // the other is the UAC elevated one. Since we are running as a service
   // as the system account we have access to both.
   TOKEN_LINKED_TOKEN tlt;
   HANDLE hNewLinkedToken = NULL;
   DWORD len;
--- a/toolkit/mozapps/update/common/updatehelper.cpp
+++ b/toolkit/mozapps/update/common/updatehelper.cpp
@@ -37,20 +37,21 @@
 
 #include <windows.h>
 #include <stdio.h>
 #include "shlobj.h"
 #include "updatehelper.h"
 
 // Needed for PathAppendW
 #include <shlwapi.h>
+// Needed for CreateToolhelp32Snapshot
+#include <tlhelp32.h>
 #pragma comment(lib, "shlwapi.lib") 
 
-WCHAR*
-MakeCommandLine(int argc, WCHAR **argv);
+WCHAR* MakeCommandLine(int argc, WCHAR **argv);
 BOOL PathAppendSafe(LPWSTR base, LPCWSTR extra);
 
 /**
  * Obtains the path of a file in the same directory as the specified file.
  *
  * @param  destinationBuffer A buffer of size MAX_PATH + 1 to store the result.
  * @param  siblingFIlePath   The path of another file in the same directory
  * @param  newFileName       The filename of another file in the same directory
@@ -253,107 +254,112 @@ StartServiceUpdate(int argc, LPWSTR *arg
   wcscpy(maintserviceInstallerPath, argv[2]);
   PathAppendSafe(maintserviceInstallerPath, 
                  L"maintenanceservice_installer.exe");
   WCHAR cmdLine[64];
   wcscpy(cmdLine, L"dummyparam.exe /Upgrade");
   BOOL svcUpdateProcessStarted = CreateProcessW(maintserviceInstallerPath, 
                                                 cmdLine, 
                                                 NULL, NULL, FALSE, 
-                                                CREATE_DEFAULT_ERROR_MODE | 
-                                                CREATE_UNICODE_ENVIRONMENT, 
+                                                0, 
                                                 NULL, argv[2], &si, &pi);
   if (svcUpdateProcessStarted) {
-    // Wait on the process to finish updating to avoid problems with 
-    // tests that are running.  maintenanceservice_installer.exe 
-    // will execute very fast anyway.
-    WaitForSingleObject(pi.hProcess, INFINITE);
     CloseHandle(pi.hProcess);
     CloseHandle(pi.hThread);
   }
   return svcUpdateProcessStarted;
 }
 
-
 /**
  * Executes a maintenance service command
  * 
  * @param  argc    The total number of arguments in argv
  * @param  argv    An array of null terminated strings to pass to the service, 
- * @return TRUE if the service command was started.
+ * @return ERROR_SUCCESS if the service command was started.
+ *         Less than 16000, a windows system error code from StartServiceW
+ *         More than 20000, 20000 + the last state of the service constant if
+ *         the last state is something other than stopped.
+ *         17001 if the SCM could not be opened
+ *         17002 if the service could not be opened
 */
-BOOL 
-StartServiceCommand(int argc, LPCWSTR* argv) 
+DWORD
+StartServiceCommand(int argc, LPCWSTR* argv)
 {
+  DWORD lastState = WaitForServiceStop(SVC_NAME, 5);
+  if (lastState != SERVICE_STOPPED) {
+    return 20000 + lastState;
+  }
+
   // Get a handle to the SCM database.
   SC_HANDLE serviceManager = OpenSCManager(NULL, NULL, 
                                            SC_MANAGER_CONNECT | 
                                            SC_MANAGER_ENUMERATE_SERVICE);
   if (!serviceManager)  {
-    return FALSE;
+    return 17001;
   }
 
   // Get a handle to the service.
   SC_HANDLE service = OpenServiceW(serviceManager, 
                                    SVC_NAME, 
-                                   SERVICE_QUERY_STATUS | SERVICE_START);
+                                   SERVICE_START);
   if (!service) {
     CloseServiceHandle(serviceManager);
-    return FALSE;
+    return 17002;
   }
 
-  // Make sure the service is not stopped.
-  SERVICE_STATUS_PROCESS ssp;
-  DWORD bytesNeeded;
-  if (!QueryServiceStatusEx(service, SC_STATUS_PROCESS_INFO, (LPBYTE)&ssp,
-                            sizeof(SERVICE_STATUS_PROCESS), &bytesNeeded)) {
-    CloseServiceHandle(service);
-    CloseServiceHandle(serviceManager);
-    return FALSE;
+  // Wait at most 5 seconds trying to start the service in case of errors
+  // like ERROR_SERVICE_DATABASE_LOCKED or ERROR_SERVICE_REQUEST_TIMEOUT.
+  const DWORD maxWaitMS = 5000;
+  DWORD currentWaitMS = 0;
+  DWORD lastError = ERROR_SUCCESS;
+  while (currentWaitMS < maxWaitMS) {
+    BOOL result = StartServiceW(service, argc, argv);
+    if (result) {
+      lastError = ERROR_SUCCESS;
+      break;
+    } else {
+      lastError = GetLastError();
+    }
+    Sleep(100);
+    currentWaitMS += 100;
   }
-
-  // The service is already in use.
-  if (ssp.dwCurrentState != SERVICE_STOPPED) {
-    CloseServiceHandle(service);
-    CloseServiceHandle(serviceManager);
-    return FALSE;
-  }
-
-  return StartServiceW(service, argc, argv);
+  CloseServiceHandle(service);
+  CloseServiceHandle(serviceManager);
+  return lastError;
 }
 
 /**
  * Launch a service initiated action for a software update with the 
  * specified arguments.
  *
  * @param  exePath The path of the executable to run
  * @param  argc    The total number of arguments in argv
  * @param  argv    An array of null terminated strings to pass to the exePath, 
  *                 argv[0] must be the path to the updater.exe
- * @return TRUE if successful
+ * @return ERROR_SUCCESS if successful
  */
-BOOL
+DWORD
 LaunchServiceSoftwareUpdateCommand(DWORD argc, LPCWSTR* argv)
 {
   // The service command is the same as the updater.exe command line except 
   // it has 2 extra args: 1) The Path to udpater.exe, and 2) the command 
   // being executed which is "software-update"
   LPCWSTR *updaterServiceArgv = new LPCWSTR[argc + 2];
-  updaterServiceArgv[0] = L"maintenanceservice.exe";
+  updaterServiceArgv[0] = L"MozillaMaintenance";
   updaterServiceArgv[1] = L"software-update";
 
   for (int i = 0; i < argc; ++i) {
     updaterServiceArgv[i + 2] = argv[i];
   }
 
   // Execute the service command by starting the service with
   // the passed in arguments.
-  BOOL result = StartServiceCommand(argc + 2, updaterServiceArgv);
+  DWORD ret = StartServiceCommand(argc + 2, updaterServiceArgv);
   delete[] updaterServiceArgv;
-  return result;
+  return ret;
 }
 
 /**
  * Joins a base directory path with a filename.
  *
  * @param  base  The base directory path of size MAX_PATH + 1
  * @param  extra The filename to append
  * @return TRUE if the file name was successful appended to base
@@ -400,17 +406,17 @@ WriteStatusPending(LPCWSTR updateDirPath
 
 /**
  * Sets update.status to a specific failure code
  *
  * @param  updateDirPath The path of the update directory
  * @return TRUE if successful
  */
 BOOL
-WriteStatusFailure(LPCWSTR updateDirPath, int errorCode) 
+WriteStatusFailure(LPCWSTR updateDirPath, int errorCode)
 {
   WCHAR updateStatusFilePath[MAX_PATH + 1];
   wcscpy(updateStatusFilePath, updateDirPath);
   if (!PathAppendSafe(updateStatusFilePath, L"update.status")) {
     return FALSE;
   }
 
   HANDLE statusFile = CreateFileW(updateStatusFilePath, GENERIC_WRITE, 0, 
@@ -429,57 +435,199 @@ WriteStatusFailure(LPCWSTR updateDirPath
   return ok && wrote == toWrite;
 }
 
 /**
  * Waits for a service to enter a stopped state.
  * This function does not stop the service, it just blocks until the service
  * is stopped.
  *
- * @param  serviceName    The service to wait for.
- * @param  maxWaitSeconds The maximum number of seconds to wait
- * @return TRUE if the service was stopped after waiting at most maxWaitSeconds
- *         FALSE on an error or when the service was not stopped
+ * @param  serviceName     The service to wait for.
+ * @param  maxWaitSeconds  The maximum number of seconds to wait
+ * @return state of the service after a timeout or when stopped.
+ *         A value of 255 is returned for an error. Typical values are:
+ *         SERVICE_STOPPED 0x00000001
+ *         SERVICE_START_PENDING 0x00000002
+ *         SERVICE_STOP_PENDING 0x00000003
+ *         SERVICE_RUNNING 0x00000004
+ *         SERVICE_CONTINUE_PENDING 0x00000005
+ *         SERVICE_PAUSE_PENDING 0x00000006
+ *         SERVICE_PAUSED 0x00000007
+ *         last status not set 0x000000CF
+ *         Could no query status 0x000000DF
+ *         Could not open service, access denied 0x000000EB
+ *         Could not open service, invalid handle 0x000000EC
+ *         Could not open service, invalid name 0x000000ED
+ *         Could not open service, does not exist 0x000000EE
+ *         Could not open service, other error 0x000000EF
+ *         Could not open SCM, access denied 0x000000FD
+ *         Could not open SCM, database does not exist 0x000000FE;
+ *         Could not open SCM, other error 0x000000FF;
+ * Note: The strange choice of error codes above SERVICE_PAUSED are chosen
+ * in case Windows comes out with other service stats higher than 7, they
+ * would likely call it 8 and above.  JS code that uses this in TestAUSHelper
+ * only handles values up to 255 so that's why we don't use GetLastError 
+ * directly.
  */
-BOOL
-WaitForServiceStop(LPCWSTR serviceName, DWORD maxWaitSeconds) 
+DWORD
+WaitForServiceStop(LPCWSTR serviceName, DWORD maxWaitSeconds)
 {
+  // 0x000000CF is defined above to be not set
+  DWORD lastServiceState = 0x000000CF;
+
   // Get a handle to the SCM database.
   SC_HANDLE serviceManager = OpenSCManager(NULL, NULL, 
                                            SC_MANAGER_CONNECT | 
                                            SC_MANAGER_ENUMERATE_SERVICE);
   if (!serviceManager)  {
-    return FALSE;
+    DWORD lastError = GetLastError();
+    switch(lastError) {
+    case ERROR_ACCESS_DENIED:
+      return 0x000000FD;
+    case ERROR_DATABASE_DOES_NOT_EXIST:
+      return 0x000000FE;
+    default:
+      return 0x000000FF;
+    }
   }
 
   // Get a handle to the service.
   SC_HANDLE service = OpenServiceW(serviceManager, 
                                    serviceName, 
                                    SERVICE_QUERY_STATUS);
   if (!service) {
+    DWORD lastError = GetLastError();
     CloseServiceHandle(serviceManager);
-    return FALSE;
+    switch(lastError) {
+    case ERROR_ACCESS_DENIED:
+      return 0x000000EB;
+    case ERROR_INVALID_HANDLE:
+      return 0x000000EC;
+    case ERROR_INVALID_NAME:
+      return 0x000000ED;
+    case ERROR_SERVICE_DOES_NOT_EXIST:
+      return 0x000000EE;
+    default:
+      return 0x000000EF;
+    } 
   }
 
-  BOOL gotStop = FALSE;
   DWORD currentWaitMS = 0;
+  SERVICE_STATUS_PROCESS ssp;
+  ssp.dwCurrentState = lastServiceState;
   while (currentWaitMS < maxWaitSeconds * 1000) {
-    // Make sure the service is not stopped.
-    SERVICE_STATUS_PROCESS ssp;
     DWORD bytesNeeded;
     if (!QueryServiceStatusEx(service, SC_STATUS_PROCESS_INFO, (LPBYTE)&ssp,
                               sizeof(SERVICE_STATUS_PROCESS), &bytesNeeded)) {
+      DWORD lastError = GetLastError();
+      switch (lastError) {
+      case ERROR_INVALID_HANDLE:
+        ssp.dwCurrentState = 0x000000D9;
+        break;
+      case ERROR_ACCESS_DENIED:
+        ssp.dwCurrentState = 0x000000DA;
+        break;
+      case ERROR_INSUFFICIENT_BUFFER:
+        ssp.dwCurrentState = 0x000000DB;
+        break;
+      case ERROR_INVALID_PARAMETER:
+        ssp.dwCurrentState = 0x000000DC;
+        break;
+      case ERROR_INVALID_LEVEL:
+        ssp.dwCurrentState = 0x000000DD;
+        break;
+      case ERROR_SHUTDOWN_IN_PROGRESS:
+        ssp.dwCurrentState = 0x000000DE;
+        break;
+      // These 3 errors can occur when the service is not yet stopped but
+      // it is stopping.
+      case ERROR_INVALID_SERVICE_CONTROL:
+      case ERROR_SERVICE_CANNOT_ACCEPT_CTRL:
+      case ERROR_SERVICE_NOT_ACTIVE:
+        currentWaitMS += 50;
+        Sleep(50);
+        continue;
+      default:
+        ssp.dwCurrentState = 0x000000DF;
+      }
+
+      // We couldn't query the status so just break out
       break;
     }
 
     // The service is already in use.
     if (ssp.dwCurrentState == SERVICE_STOPPED) {
-      gotStop = TRUE;
       break;
     }
     currentWaitMS += 50;
     Sleep(50);
   }
 
+  lastServiceState = ssp.dwCurrentState;
   CloseServiceHandle(service);
   CloseServiceHandle(serviceManager);
-  return gotStop;
+  return lastServiceState;
 }
+
+/**
+ * Determines if there is at least one process running for the specified
+ * application. A match will be found across any session for any user.
+ *
+ * @param process The process to check for existance
+ * @return ERROR_NOT_FOUND if the process was not found
+ * @       ERROR_SUCCESS if the process was found and there were no errors
+ * @       Other Win32 system error code for other errors
+**/
+DWORD
+IsProcessRunning(LPCWSTR filename)
+{
+  // Take a snapshot of all processes in the system.
+  HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
+  if (INVALID_HANDLE_VALUE == snapshot) {
+    return GetLastError();
+  }
+  
+  PROCESSENTRY32W processEntry;
+  processEntry.dwSize = sizeof(PROCESSENTRY32W);
+  if (!Process32FirstW(snapshot, &processEntry)) {
+    DWORD lastError = GetLastError();
+    CloseHandle(snapshot);
+    return lastError;
+  }
+
+  do {
+    if (wcsicmp(filename, processEntry.szExeFile) == 0) {
+      CloseHandle(snapshot);
+      return ERROR_SUCCESS;
+    }
+  } while (Process32NextW(snapshot, &processEntry));
+  CloseHandle(snapshot);
+  return ERROR_NOT_FOUND;
+}
+
+/**
+ * Waits for the specified applicaiton to exit.
+ *
+ * @param filename   The application to wait for.
+ * @param maxSeconds The maximum amount of seconds to wait for all
+ *                   instances of the application to exit.
+ * @return  ERROR_SUCCESS if no instances of the application exist
+ *          WAIT_TIMEOUT if the process is still running after maxSeconds.
+ *          Any other Win32 system error code.
+*/
+DWORD
+WaitForProcessExit(LPCWSTR filename, DWORD maxSeconds)
+{
+  DWORD applicationRunningError = WAIT_TIMEOUT;
+  for(DWORD i = 0; i < maxSeconds; i++) {
+    DWORD applicationRunningError = IsProcessRunning(filename);
+    if (ERROR_NOT_FOUND == applicationRunningError) {
+      return ERROR_SUCCESS;
+    }
+    Sleep(1000);
+  }
+
+  if (ERROR_SUCCESS == applicationRunningError) {
+    return WAIT_TIMEOUT;
+  }
+
+  return applicationRunningError;
+}
--- a/toolkit/mozapps/update/common/updatehelper.h
+++ b/toolkit/mozapps/update/common/updatehelper.h
@@ -36,13 +36,15 @@
  * ***** END LICENSE BLOCK ***** */
 
 BOOL LaunchWinPostProcess(const WCHAR *installationDir,
                           const WCHAR *updateInfoDir, 
                           bool forceSync,
                           HANDLE userToken);
 BOOL StartServiceUpdate(int argc, LPWSTR *argv);
 BOOL GetUpdateDirectoryPath(LPWSTR path);
-BOOL LaunchServiceSoftwareUpdateCommand(DWORD argc, LPCWSTR *argv);
+DWORD LaunchServiceSoftwareUpdateCommand(DWORD argc, LPCWSTR *argv);
 BOOL WriteStatusFailure(LPCWSTR updateDirPath, int errorCode);
 BOOL WriteStatusPending(LPCWSTR updateDirPath);
-BOOL WaitForServiceStop(LPCWSTR serviceName, DWORD maxWaitSeconds);
+DWORD WaitForServiceStop(LPCWSTR serviceName, DWORD maxWaitSeconds);
+DWORD WaitForProcessExit(LPCWSTR filename, DWORD maxSeconds);
+
 #define SVC_NAME L"MozillaMaintenance"
--- a/toolkit/mozapps/update/nsUpdateService.js
+++ b/toolkit/mozapps/update/nsUpdateService.js
@@ -143,16 +143,18 @@ const STATE_FAILED          = "failed";
 const WRITE_ERROR        = 7;
 const UNEXPECTED_ERROR   = 8;
 const ELEVATION_CANCELED = 9;
 const SERVICE_UPDATER_COULD_NOT_BE_STARTED = 16000;
 const SERVICE_NOT_ENOUGH_COMMAND_LINE_ARGS = 16001;
 const SERVICE_UPDATER_SIGN_ERROR           = 16002;
 const SERVICE_UPDATER_COMPARE_ERROR        = 16003;
 const SERVICE_UPDATER_IDENTITY_ERROR       = 16004;
+const SERVICE_STILL_APPLYING_ON_SUCCESS    = 16005;
+const SERVICE_STILL_APPLYING_ON_FAILURE    = 16006;
 
 const CERT_ATTR_CHECK_FAILED_NO_UPDATE  = 100;
 const CERT_ATTR_CHECK_FAILED_HAS_UPDATE = 101;
 const BACKGROUNDCHECK_MULTIPLE_FAILURES = 110;
 
 const DOWNLOAD_CHUNK_SIZE           = 300000; // bytes
 const DOWNLOAD_BACKGROUND_INTERVAL  = 600;    // seconds
 const DOWNLOAD_FOREGROUND_INTERVAL  = 0;
@@ -1434,17 +1436,19 @@ UpdateService.prototype = {
           writeStatusFile(getUpdatesDir(), update.state = STATE_PENDING);
           return;
         }
 
         if (update.errorCode == SERVICE_UPDATER_COULD_NOT_BE_STARTED ||
             update.errorCode == SERVICE_NOT_ENOUGH_COMMAND_LINE_ARGS ||
             update.errorCode == SERVICE_UPDATER_SIGN_ERROR ||
             update.errorCode == SERVICE_UPDATER_COMPARE_ERROR ||
-            update.errorCode == SERVICE_UPDATER_IDENTITY_ERROR) {
+            update.errorCode == SERVICE_UPDATER_IDENTITY_ERROR ||
+            update.errorCode == SERVICE_STILL_APPLYING_ON_SUCCESS ||
+            update.errorCode == SERVICE_STILL_APPLYING_ON_FAILURE) {
           var failCount = getPref("getIntPref", 
                                   PREF_APP_UPDATE_SERVICE_ERRORS, 0);
           var maxFail = getPref("getIntPref", 
                                 PREF_APP_UPDATE_SERVICE_MAX_ERRORS, 
                                 DEFAULT_SERVICE_MAX_ERRORS);
 
           // As a safety, when the service reaches maximum failures, it will
           // disable itself and fallback to using the normal update mechanism
--- a/toolkit/mozapps/update/test/TestAUSHelper.cpp
+++ b/toolkit/mozapps/update/test/TestAUSHelper.cpp
@@ -2,16 +2,17 @@
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 
 #ifdef XP_WIN
 #pragma comment(lib, "wintrust.lib")
 #pragma comment(lib, "crypt32.lib")
 # include <windows.h>
 # include <wintrust.h>
+# include <tlhelp32.h>
 # include <softpub.h>
 # include <direct.h>
 # include <io.h>
   typedef WCHAR NS_tchar;
 # define NS_main wmain
 # define F_OK 00
 # define W_OK 02
 # define R_OK 04
@@ -152,63 +153,216 @@ VerifyCertificateTrustForFile(LPCWSTR fi
   return WinVerifyTrust(NULL, &policyGUID, &trustData);
 }
 
 /**
  * Waits for a service to enter a stopped state.
  * This function does not stop the service, it just blocks until the service
  * is stopped.
  *
- * @param  serviceName    The service to wait for.
- * @param  maxWaitSeconds The maximum number of seconds to wait
- * @return true if the service was stopped after waiting at most maxWaitSeconds
- *         false on an error or when the service was not stopped
+ * @param  serviceName     The service to wait for.
+ * @param  maxWaitSeconds  The maximum number of seconds to wait
+ * @return state of the service after a timeout or when stopped.
+ *         A value of 255 is returned for an error. Typical values are:
+ *         SERVICE_STOPPED 0x00000001
+ *         SERVICE_START_PENDING 0x00000002
+ *         SERVICE_STOP_PENDING 0x00000003
+ *         SERVICE_RUNNING 0x00000004
+ *         SERVICE_CONTINUE_PENDING 0x00000005
+ *         SERVICE_PAUSE_PENDING 0x00000006
+ *         SERVICE_PAUSED 0x00000007
+ *         last status not set 0x000000CF
+ *         Could no query status 0x000000DF
+ *         Could not open service, access denied 0x000000EB
+ *         Could not open service, invalid handle 0x000000EC
+ *         Could not open service, invalid name 0x000000ED
+ *         Could not open service, does not exist 0x000000EE
+ *         Could not open service, other error 0x000000EF
+ *         Could not open SCM, access denied 0x000000FD
+ *         Could not open SCM, database does not exist 0x000000FE;
+ *         Could not open SCM, other error 0x000000FF;
+ * Note: The strange choice of error codes above SERVICE_PAUSED are chosen
+ * in case Windows comes out with other service stats higher than 7, they
+ * would likely call it 8 and above.  JS code that uses this in TestAUSHelper
+ * only handles values up to 255 so that's why we don't use GetLastError 
+ * directly.
  */
-bool WaitForServiceStop(LPCWSTR serviceName, DWORD maxWaitSeconds) 
+DWORD
+WaitForServiceStop(LPCWSTR serviceName, DWORD maxWaitSeconds)
 {
+  // 0x000000CF is defined above to be not set
+  DWORD lastServiceState = 0x000000CF;
+
   // Get a handle to the SCM database.
   SC_HANDLE serviceManager = OpenSCManager(NULL, NULL, 
                                            SC_MANAGER_CONNECT | 
                                            SC_MANAGER_ENUMERATE_SERVICE);
   if (!serviceManager)  {
-    return false;
+    DWORD lastError = GetLastError();
+    switch(lastError) {
+    case ERROR_ACCESS_DENIED:
+      return 0x000000FD;
+    case ERROR_DATABASE_DOES_NOT_EXIST:
+      return 0x000000FE;
+    default:
+      return 0x000000FF;
+    }
   }
 
   // Get a handle to the service.
   SC_HANDLE service = OpenServiceW(serviceManager, 
                                    serviceName, 
                                    SERVICE_QUERY_STATUS);
   if (!service) {
+    DWORD lastError = GetLastError();
     CloseServiceHandle(serviceManager);
-    return false;
+    switch(lastError) {
+    case ERROR_ACCESS_DENIED:
+      return 0x000000EB;
+    case ERROR_INVALID_HANDLE:
+      return 0x000000EC;
+    case ERROR_INVALID_NAME:
+      return 0x000000ED;
+    // If the service does not exist, keep trying in case it does exist soon.
+    // I think there might be an issue with the TALOS machines and some of 
+    // the machines having an old maintenanceservice.exe that used to 
+    // uninstall when upgrading.  Those should already be upgraded but this 
+    // is safer.
+    case ERROR_SERVICE_DOES_NOT_EXIST:
+      if (maxWaitSeconds == 0) {
+        return 0x000000EE;
+      } else {
+        Sleep(1000);
+        return WaitForServiceStop(serviceName, maxWaitSeconds - 1);
+      }
+    default:
+      return 0x000000EF;
+    } 
   }
 
-  bool gotStop = false;
   DWORD currentWaitMS = 0;
+  SERVICE_STATUS_PROCESS ssp;
+  ssp.dwCurrentState = lastServiceState;
   while (currentWaitMS < maxWaitSeconds * 1000) {
-    // Make sure the service is not stopped.
-    SERVICE_STATUS_PROCESS ssp;
     DWORD bytesNeeded;
     if (!QueryServiceStatusEx(service, SC_STATUS_PROCESS_INFO, (LPBYTE)&ssp,
                               sizeof(SERVICE_STATUS_PROCESS), &bytesNeeded)) {
+      DWORD lastError = GetLastError();
+      switch (lastError) {
+      case ERROR_INVALID_HANDLE:
+        ssp.dwCurrentState = 0x000000D9;
+        break;
+      case ERROR_ACCESS_DENIED:
+        ssp.dwCurrentState = 0x000000DA;
+        break;
+      case ERROR_INSUFFICIENT_BUFFER:
+        ssp.dwCurrentState = 0x000000DB;
+        break;
+      case ERROR_INVALID_PARAMETER:
+        ssp.dwCurrentState = 0x000000DC;
+        break;
+      case ERROR_INVALID_LEVEL:
+        ssp.dwCurrentState = 0x000000DD;
+        break;
+      case ERROR_SHUTDOWN_IN_PROGRESS:
+        ssp.dwCurrentState = 0x000000DE;
+        break;
+      // These 3 errors can occur when the service is not yet stopped but
+      // it is stopping.
+      case ERROR_INVALID_SERVICE_CONTROL:
+      case ERROR_SERVICE_CANNOT_ACCEPT_CTRL:
+      case ERROR_SERVICE_NOT_ACTIVE:
+        currentWaitMS += 50;
+        Sleep(50);
+        continue;
+      default:
+        ssp.dwCurrentState = 0x000000DF;
+      }
+
+      // We couldn't query the status so just break out
       break;
     }
 
     // The service is already in use.
     if (ssp.dwCurrentState == SERVICE_STOPPED) {
-      gotStop = true;
       break;
     }
     currentWaitMS += 50;
     Sleep(50);
   }
 
+  lastServiceState = ssp.dwCurrentState;
   CloseServiceHandle(service);
   CloseServiceHandle(serviceManager);
-  return gotStop;
+  return lastServiceState;
+}
+
+/**
+ * Determines if there is at least one process running for the specified
+ * application. A match will be found across any session for any user.
+ *
+ * @param process The process to check for existance
+ * @return ERROR_NOT_FOUND if the process was not found
+ * @       ERROR_SUCCESS if the process was found and there were no errors
+ * @       Other Win32 system error code for other errors
+**/
+DWORD
+IsProcessRunning(LPCWSTR filename)
+{
+  // Take a snapshot of all processes in the system.
+  HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
+  if (INVALID_HANDLE_VALUE == snapshot) {
+    return GetLastError();
+  }
+  
+  PROCESSENTRY32W processEntry;
+  processEntry.dwSize = sizeof(PROCESSENTRY32W);
+  if (!Process32FirstW(snapshot, &processEntry)) {
+    DWORD lastError = GetLastError();
+    CloseHandle(snapshot);
+    return lastError;
+  }
+
+  do {
+    if (wcsicmp(filename, processEntry.szExeFile) == 0) {
+      CloseHandle(snapshot);
+      return ERROR_SUCCESS;
+    }
+  } while (Process32NextW(snapshot, &processEntry));
+  CloseHandle(snapshot);
+  return ERROR_NOT_FOUND;
+}
+
+/**
+ * Waits for the specified applicaiton to exit.
+ *
+ * @param filename   The application to wait for.
+ * @param maxSeconds The maximum amount of seconds to wait for all
+ *                   instances of the application to exit.
+ * @return  ERROR_SUCCESS if no instances of the application exist
+ *          WAIT_TIMEOUT if the process is still running after maxSeconds.
+ *          Any other Win32 system error code.
+*/
+DWORD
+WaitForProcessExit(LPCWSTR filename, DWORD maxSeconds)
+{
+  DWORD applicationRunningError = WAIT_TIMEOUT;
+  for(DWORD i = 0; i < maxSeconds; i++) {
+    DWORD applicationRunningError = IsProcessRunning(filename);
+    if (ERROR_NOT_FOUND == applicationRunningError) {
+      return ERROR_SUCCESS;
+    }
+    Sleep(1000);
+  }
+
+  if (ERROR_SUCCESS == applicationRunningError) {
+    return WAIT_TIMEOUT;
+  }
+
+  return applicationRunningError;
 }
 
 
 #endif
 
 int NS_main(int argc, NS_tchar **argv)
 {
 
@@ -253,20 +407,39 @@ int NS_main(int argc, NS_tchar **argv)
     return 1;
 #endif
   }
 
   if (!NS_tstrcmp(argv[1], NS_T("wait-for-service-stop"))) {
 #ifdef XP_WIN
     const int maxWaitSeconds = NS_ttoi(argv[3]);
     LPCWSTR serviceName = argv[2];
-    if (WaitForServiceStop(serviceName, maxWaitSeconds)) {
+    DWORD serviceState = WaitForServiceStop(serviceName, maxWaitSeconds);
+    if (SERVICE_STOPPED == serviceState) {
       return 0;
     } else {
+      return serviceState;
+    }
+#else 
+    // Not implemented on non-Windows platforms
+    return 1;
+#endif
+  }
+
+  if (!NS_tstrcmp(argv[1], NS_T("wait-for-application-exit"))) {
+#ifdef XP_WIN
+    const int maxWaitSeconds = NS_ttoi(argv[3]);
+    LPCWSTR application = argv[2];
+    DWORD ret = WaitForProcessExit(application, maxWaitSeconds);
+    if (ERROR_SUCCESS == ret) {
+      return 0;
+    } else if (WAIT_TIMEOUT == ret) {
       return 1;
+    } else {
+      return 2;
     }
 #else 
     // Not implemented on non-Windows platforms
     return 1;
 #endif
   }
 
   int i = 0;
--- a/toolkit/mozapps/update/test/unit/head_update.js.in
+++ b/toolkit/mozapps/update/test/unit/head_update.js.in
@@ -457,17 +457,22 @@ let gServiceLaunchedCallbackLog = null;
 let gServiceLaunchedCallbackArgs = null;
 
 /**
  * Helper function to check whether the maintenance service updater tests should
  * run. See bug 711660 for more details.
  *
  * @return true if the test should run and false if it shouldn't.
  */
-function shouldRunServiceTest() {
+function shouldRunServiceTest(aFirstTest) {
+  // In case the machine is running an old maintenance service or if it
+  // is not installed, and permissions exist to install it.  Then install
+  // the newer bin that we have.
+  attemptServiceInstall();
+
   const REG_PATH = "SOFTWARE\\Mozilla\\MaintenanceService\\" +
                    "3932ecacee736d366d6436db0f55bce4";
 
   let key = AUS_Cc["@mozilla.org/windows-registry-key;1"].
             createInstance(AUS_Ci.nsIWindowsRegKey);
   try {
     key.open(AUS_Ci.nsIWindowsRegKey.ROOT_KEY_LOCAL_MACHINE, REG_PATH,
              AUS_Ci.nsIWindowsRegKey.ACCESS_READ | key.WOW64_64);
@@ -485,27 +490,48 @@ function shouldRunServiceTest() {
     do_throw("Unable to find updater binary!");
   }
 
   let updaterBinPath = updaterBin.path;
   if (/ /.test(updaterBinPath)) {
     updaterBinPath = '"' + updaterBinPath + '"';
   }
 
+  // Check to make sure the service is installed
   let helperBin = do_get_file(HELPER_BIN_FILE);
-  let args = ["check-signature", updaterBinPath];
+  let args = ["wait-for-service-stop", "MozillaMaintenance", "10"];
   let process = AUS_Cc["@mozilla.org/process/util;1"].
                 createInstance(AUS_Ci.nsIProcess);
   process.init(helperBin);
+  logTestInfo("Checking if the service exists on this machine.");
   process.run(true, args, args.length);
+  if (process.exitValue == 0xEE) {
+    logTestInfo("this test can only run when the service is installed.");
+    return false;
+  } else {
+    logTestInfo("Service exists, return value: " + process.exitValue);
+  }
 
+  // If this is the first test in the series, then there is no reason the
+  // service should be anything but stopped, so be strict here and throw
+  // an error.
+  if (aFirstTest && process.exitValue != 0) {
+    do_throw("First test, check for service stopped state returned error " + 
+             process.exitValue);
+  }
+
+  // Make sure the binaries are signed
+  args = ["check-signature", updaterBinPath];
+  process = AUS_Cc["@mozilla.org/process/util;1"].
+            createInstance(AUS_Ci.nsIProcess);
+  process.init(helperBin);
+  process.run(true, args, args.length);
   if (process.exitValue == 0) {
     return true;
   }
-
   logTestInfo("this test can only run on builds with signed binaries. " +
               HELPER_BIN_FILE + " returned " + process.exitValue)
   return false;
 }
 
 /**
  * Copies the specified filename from the dist/bin
  * directory into the apply-to directory.
@@ -522,16 +548,44 @@ function copyBinToApplyToDir(filename) {
   let applyToUpdater = getApplyDirFile(null, true);
   if (applyToUpdater.path != binDir.path) {
     do_print("copying " + fileToCopy.path + " to: " + applyToUpdater.path);
     fileToCopy.copyTo(applyToUpdater, filename);
   }
 }
 
 /**
+ * Attempts to upgrade the maintenance service if permissions are allowed.
+ * This is useful for XP where we have permission to upgrade in case an
+ * older service installer exists.  Also if the user manually installed into
+ * a unprivileged location.
+*/
+function attemptServiceInstall() {
+  var version = AUS_Cc["@mozilla.org/system-info;1"]
+                .getService(AUS_Ci.nsIPropertyBag2)
+                .getProperty("version");
+  var isVistaOrHigher = (parseFloat(version) >= 6.0);
+  if (isVistaOrHigher) {
+    return;
+  }
+  
+  let binDir = getGREDir();
+  let installerFile = binDir.clone();
+  installerFile.append(MAINTENANCE_SERVICE_INSTALLER_BIN_FILE);
+  if (!installerFile.exists()) {
+    do_throw(MAINTENANCE_SERVICE_INSTALLER_BIN_FILE + " not found.");
+  }
+  let installerProcess = AUS_Cc["@mozilla.org/process/util;1"].
+                         createInstance(AUS_Ci.nsIProcess);
+  installerProcess.init(installerFile);
+  logTestInfo("Starting installer process...");
+  installerProcess.run(true, [], 0);
+}
+
+/**
  * Helper function for updater tests for launching the updater using the
  * maintenance service to apply a mar file.
  *
  * @param aInitialStatus  the initial value of update.status
  * @param aExpectedStatus the expected value of update.status when the test finishes
  * @param aCallback       the function to be called when the update is finished
  * @param aUpdatesDir     the updates root directory to use (optional)
  * @param aCheckSvcLog    whether the service log should be checked (optional)
@@ -549,62 +603,83 @@ function runUpdateUsingService(aInitialS
     let file = AUS_Cc["@mozilla.org/file/directory_service;1"].
                getService(AUS_Ci.nsIProperties).
                get("CmAppData", AUS_Ci.nsIFile);
     file.append("Mozilla");
     file.append("logs");
     file.append("maintenanceservice.log");
     return readFile(file);
   }
+  function waitServiceApps() {
+    // maintenanceservice_installer.exe is started async during updates.
+    waitForApplicationStop("maintenanceservice_installer.exe");
+    // maintenanceservice_tmp.exe is started async from the service installer.
+    waitForApplicationStop("maintenanceservice_tmp.exe");
+    // In case the SCM thinks the service is stopped, but process still exists.
+    waitForApplicationStop("maintenanceservice.exe");
+  }
   function waitForServiceStop(aFailTest) {
+    waitServiceApps(); 
     logTestInfo("Waiting for service to stop if necessary...");
     // Use the helper bin to ensure the service is stopped. If not
-    // stopped then wait for the service to be stopped (at most 20 seconds)
+    // stopped then wait for the service to be stopped (at most 120 seconds)
     let helperBin = do_get_file(HELPER_BIN_FILE);
     let helperBinArgs = ["wait-for-service-stop", 
                          "MozillaMaintenance", 
-                         "20"];
+                         "120"];
+    let helperBinProcess = AUS_Cc["@mozilla.org/process/util;1"].
+                           createInstance(AUS_Ci.nsIProcess);
+    helperBinProcess.init(helperBin);
+    logTestInfo("Stopping service...");
+    helperBinProcess.run(true, helperBinArgs, helperBinArgs.length);
+    if (helperBinProcess.exitValue == 0xEE) {
+      do_throw("The service does not exist on this machine.  Return value: " +
+               helperBinProcess.exitValue);
+    } else if (helperBinProcess.exitValue != 0) {
+      if (aFailTest) {
+        do_throw("maintenance service did not stop, last state: " +
+                 helperBinProcess.exitValue + ". Forcing test failure.");
+      } else {
+        logTestInfo("maintenance service did not stop, last state: " + 
+                    helperBinProcess.exitValue + ".  May cause failures.");
+      }
+    } else {
+      logTestInfo("Service stopped.");
+    }
+    waitServiceApps(); 
+  }
+  function waitForApplicationStop(application) {
+    logTestInfo("Waiting for " + application + " to stop if " + 
+                "necessary...");
+    // Use the helper bin to ensure the application is stopped. 
+    // If not, then wait for it to be stopped (at most 120 seconds)
+    let helperBin = do_get_file(HELPER_BIN_FILE);
+    let helperBinArgs = ["wait-for-application-exit", 
+                         application, 
+                         "120"];
     let helperBinProcess = AUS_Cc["@mozilla.org/process/util;1"].
                            createInstance(AUS_Ci.nsIProcess);
     helperBinProcess.init(helperBin);
     helperBinProcess.run(true, helperBinArgs, helperBinArgs.length);
     if (helperBinProcess.exitValue != 0) {
-      if (aFailTest) {
-        do_throw("maintenance service did not stop, forcing test failure.");
-      } else {
-        logTestInfo("maintenance service did not stop, may cause failures.");
-      }
-    } else {
-      logTestInfo("Service stopped.");
+      do_throw(application + " did not stop, last state: " +
+               helperBinProcess.exitValue + ". Forcing test failure.");
     }
   }
 
   // Make sure the service from the previous test is already stopped.
   waitForServiceStop(true);
 
   // Prevent the cleanup function from begin run more than once
   if (typeof(gRegisteredServiceCleanup) === "undefined") {
     gRegisteredServiceCleanup = true;
 
     do_register_cleanup(function serviceCleanup() {
       resetEnvironment();
 
-      // Remove the copy of the application executable used for the test on
-      // Windows if it exists.
-      let appBinCopy = getCurrentProcessDir();
-      appBinCopy.append(TEST_ID + FILE_WIN_TEST_EXE);
-      if (appBinCopy.exists()) {
-        try {
-          appBinCopy.remove(false);
-        }
-        catch (e) {
-          logTestInfo("unable to remove file during cleanup. Exception: " + e);
-        }
-      }
-
       // This will delete the app console log file if it exists.
       try {
         getAppConsoleLogPath();
       }
       catch (e) {
         logTestInfo("unable to remove file during cleanup. Exception: " + e);
       }
 
@@ -667,17 +742,17 @@ function runUpdateUsingService(aInitialS
   // The service will execute maintenanceservice_installer.exe and
   // will copy maintenanceservice.exe out of the same directory from
   // the installation directory.  So we need to make sure both of those
   // bins always exist in the installation directory.
   copyBinToApplyToDir(MAINTENANCE_SERVICE_BIN_FILE);
   copyBinToApplyToDir(MAINTENANCE_SERVICE_INSTALLER_BIN_FILE);
 
   // Firefox does not wait for the service command to finish, but
-  // we still launch the process sync to avoid intemittent failures with
+  // we still launch the process sync to avoid intermittent failures with
   // the log file not being written out yet.
   // We will rely on watching the update.status file and waiting for the service
   // to stop to know the service command is done.
   process.run(true, args, args.length);
 
   resetEnvironment();
 
   function timerCallback(timer) {
@@ -691,24 +766,25 @@ function runUpdateUsingService(aInitialS
     // race condition where it would be possible on slower machines where status
     // could be equal to STATE_PENDING_SVC.
     if (status == STATE_APPLYING || 
         status == STATE_PENDING_SVC) {
       logTestInfo("Still waiting to see the " + aExpectedStatus +
                   " status, got " + status + " for now...");
       return;
     }
+    
+    // Make sure all of the logs are written out.
+    waitForServiceStop(false);
+
     do_check_eq(status, aExpectedStatus);
 
     timer.cancel();
     timer = null;
 
-    // Make sure all of the logs are written out.
-    waitForServiceStop(false);
-
     if (aCheckSvcLog) {
       checkServiceLogs(svcOriginalLog);
     } 
     aCallback();
   }
 
   let timer = AUS_Cc["@mozilla.org/timer;1"].createInstance(AUS_Ci.nsITimer);
   timer.initWithCallback(timerCallback, 1000, timer.TYPE_REPEATING_SLACK);
@@ -1201,18 +1277,40 @@ function checkCallbackServiceLog() {
     do_timeout(TEST_HELPER_TIMEOUT, checkCallbackServiceLog);
     return;
   }
 
   logTestInfo("testing that the callback application successfully launched " +
               "and the expected command line arguments passed to it");
   do_check_eq(logContents, expectedLogContents);
 
-  // Use a timeout to give any files that were in use additional time to close.
-  do_timeout(TEST_HELPER_TIMEOUT, do_test_finished);
+
+  (function removeCallbackCopy() {
+    // Remove the copy of the application executable used for the test on
+    // Windows if it exists.
+    let appBinCopy = getCurrentProcessDir();
+    appBinCopy.append(TEST_ID + FILE_WIN_TEST_EXE);
+    if (appBinCopy.exists()) {
+      try {
+        appBinCopy.remove(false);
+        // Use a timeout to give any files that were in use additional 
+        // time to close.  Same as updater.exe without service tests.
+        do_timeout(TEST_HELPER_TIMEOUT, do_test_finished);
+      }
+      catch (e) {
+        logTestInfo("unable to remove file during cleanup. Exception: " + e);
+        do_timeout(TEST_HELPER_TIMEOUT, removeCallbackCopy);
+      }
+    } else {
+      do_timeout(TEST_HELPER_TIMEOUT, do_test_finished);
+    }
+  })();
+
+
+
 }
 
 /**
  * Helper function for updater binary tests for verifying there are no update
  * backup files left behind after an update.
  *
  * @param   aFile
  *          An nsIFile to check if it has moz-backup for its extension.
--- a/toolkit/mozapps/update/test_svc/unit/test_0000_bootstrap_svc.js
+++ b/toolkit/mozapps/update/test_svc/unit/test_0000_bootstrap_svc.js
@@ -16,17 +16,17 @@ const TEST_FILES = [
   originalFile     : null,
   compareFile      : null,
   originalPerms    : null,
   comparePerms     : null
 }
 ];
 
 function run_test() {
-  if (!shouldRunServiceTest()) {
+  if (!shouldRunServiceTest(true)) {
     return;
   }
 
   do_test_pending();
   do_register_cleanup(cleanupUpdaterTest);
 
   setupUpdaterTest(MAR_COMPLETE_FILE);
 
--- a/toolkit/mozapps/update/updater/updater.cpp
+++ b/toolkit/mozapps/update/updater/updater.cpp
@@ -1600,18 +1600,18 @@ int NS_main(int argc, NS_tchar **argv)
 
 #ifdef XP_WIN
   // Disable every privilege we don't need. Processes started using
   // CreateProcess will use the same token as this process.
   UACHelper::DisablePrivileges(NULL);
 
   bool useService = false;
   bool testOnlyFallbackKeyExists = false;
-  bool noServiceFallback = _wgetenv(L"MOZ_NO_SERVICE_FALLBACK") != NULL;
-  _wputenv(L"MOZ_NO_SERVICE_FALLBACK=");
+  bool noServiceFallback = getenv("MOZ_NO_SERVICE_FALLBACK") != NULL;
+  putenv(const_cast<char*>("MOZ_NO_SERVICE_FALLBACK="));
 
   // We never want the service to be used unless we build with
   // the maintenance service.
 #ifdef MOZ_MAINTENANCE_SERVICE
   IsUpdateStatusPending(useService);
   // Our tests run with a different apply directory for each test.
   // We use this registry key on our test slaves to store the 
   // allowed name/issuers.
@@ -1675,19 +1675,20 @@ int NS_main(int argc, NS_tchar **argv)
 #endif
   }
 
   // The callback is the remaining arguments starting at callbackIndex.
   // The argument specified by callbackIndex is the callback executable and the
   // argument prior to callbackIndex is the working directory.
   const int callbackIndex = 5;
 
+  bool usingService = false;
 #if defined(XP_WIN)
-  bool usingService = _wgetenv(L"MOZ_USING_SERVICE") != NULL;
-  _wputenv(L"MOZ_USING_SERVICE=");
+  usingService = getenv("MOZ_USING_SERVICE") != NULL;
+  putenv(const_cast<char*>("MOZ_USING_SERVICE="));
   // lastFallbackError keeps track of the last error for the service not being 
   // used, in case of an error when fallback is not enabled we write the 
   // error to the update.status file. 
   // When fallback is disabled (MOZ_NO_SERVICE_FALLBACK does not exist) then
   // we will instead fallback to not using the service and display a UAC prompt.
   int lastFallbackError = FALLBACKKEY_UNKNOWN_ERROR;
 
   // Launch a second instance of the updater with the runas verb on Windows
@@ -1778,26 +1779,29 @@ int NS_main(int argc, NS_tchar **argv)
       // fails in between, we can fall back to using the normal update process
       // on our own.
 
       // If we still want to use the service try to launch the service 
       // comamnd for the update.
       if (useService) {
         // If the update couldn't be started, then set useService to false so
         // we do the update the old way.
-        useService = LaunchServiceSoftwareUpdateCommand(argc, (LPCWSTR *)argv);
-
+        DWORD ret = LaunchServiceSoftwareUpdateCommand(argc, (LPCWSTR *)argv);
+        useService = (ret == ERROR_SUCCESS);
         // If the command was launched then wait for the service to be done.
         if (useService) {
-          if (!WaitForServiceStop(SVC_NAME, 600)) {
+          DWORD lastState = WaitForServiceStop(SVC_NAME, 600);
+          if (lastState != SERVICE_STOPPED) {
             // If the service doesn't stop after 10 minutes there is
             // something seriously wrong.
             lastFallbackError = FALLBACKKEY_SERVICE_NO_STOP_ERROR;
             useService = false;
           }
+        } else {
+          lastFallbackError = FALLBACKKEY_LAUNCH_ERROR;
         }
       }
 
       // If we started the service command, and it finished, check the
       // update.status file to make sure it succeeded, and if it did
       // we need to manually start the PostUpdate process from the
       // current user's session of this unelevated updater.exe the
       // current process is running as.
@@ -1898,18 +1902,22 @@ int NS_main(int argc, NS_tchar **argv)
   *c = NS_T('\0');
   c++;
 
   gDestPath = destpath;
 
   HANDLE callbackFile = INVALID_HANDLE_VALUE;
   if (argc > callbackIndex) {
     // If the callback executable is specified it must exist for a successful
-    // update.
+    // update.  It is important we null out the whole buffer here because later
+    // we make the assumption that the callback application is inside the
+    // apply-to dir.  If we don't have a fully null'ed out buffer it can lead
+    // to stack corruption which causes crashes and other problems.
     NS_tchar callbackLongPath[MAXPATHLEN];
+    ZeroMemory(callbackLongPath, sizeof(callbackLongPath));
     if (!GetLongPathNameW(argv[callbackIndex], callbackLongPath,
                           sizeof(callbackLongPath)/sizeof(callbackLongPath[0]))) {
       LOG(("NS_main: unable to find callback file: " LOG_S "\n", argv[callbackIndex]));
       LogFinish();
       WriteStatusFile(WRITE_ERROR);
       EXIT_WHEN_ELEVATED(elevatedLockFilePath, updateLockFileHandle, 1);
       LaunchCallbackApp(argv[4], 
                         argc - callbackIndex, 
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -3131,46 +3131,51 @@ XRE_main(int argc, char* argv[], const n
     nsCOMPtr<nsIFile> updRoot;
     bool persistent;
     rv = dirProvider.GetFile(XRE_UPDATE_ROOT_DIR, &persistent,
                              getter_AddRefs(updRoot));
     // XRE_UPDATE_ROOT_DIR may fail. Fallback to appDir if failed
     if (NS_FAILED(rv))
       updRoot = dirProvider.GetAppDir();
 
+    // If the MOZ_PROCESS_UPDATES environment variable already exists, then
+    // we are being called from the callback application.
+    if (EnvHasValue("MOZ_PROCESS_UPDATES")) {
+      // If the caller has asked us to log our arguments, do so.  This is used
+      // to make sure that the maintenance service successfully launches the
+      // callback application.
+      const char *logFile = nsnull;
+      if (ARG_FOUND == CheckArg("dump-args", false, &logFile)) {
+        FILE* logFP = fopen(logFile, "wb");
+        if (logFP) {
+          for (i = 1; i < gRestartArgc; ++i) {
+            fprintf(logFP, "%s\n", gRestartArgv[i]);
+          }
+          fclose(logFP);
+        }
+      }
+      return 0;
+    }
+
     // Support for processing an update and exiting. The MOZ_PROCESS_UPDATES
     // environment variable will be part of the updater's environment and the
     // application that is relaunched by the updater. When the application is
     // relaunched by the updater it will be removed below and the application
     // will exit.
     if (CheckArg("process-updates")) {
       SaveToEnv("MOZ_PROCESS_UPDATES=1");
     }
     ProcessUpdates(dirProvider.GetGREDir(),
                    dirProvider.GetAppDir(),
                    updRoot,
                    gRestartArgc,
                    gRestartArgv,
                    appData.version);
     if (EnvHasValue("MOZ_PROCESS_UPDATES")) {
       SaveToEnv("MOZ_PROCESS_UPDATES=");
-
-      // If the caller has asked us to log our arguments, do so.  This is used
-      // to make sure that the maintenance service successfully launches the
-      // callback application.
-      const char *logFile = nsnull;
-      if (ARG_FOUND == CheckArg("dump-args", false, &logFile)) {
-        FILE* logFP = fopen(logFile, "wb");
-        if (logFP) {
-          for (i = 1; i < gRestartArgc; ++i) {
-            fprintf(logFP, "%s\n", gRestartArgv[i]);
-          }
-          fclose(logFP);
-        }
-      }
       return 0;
     }
 #endif
 
     nsCOMPtr<nsIProfileLock> profileLock;
     bool startOffline = false;
     nsCAutoString profileName;
 
--- a/toolkit/xre/nsWindowsRestart.cpp
+++ b/toolkit/xre/nsWindowsRestart.cpp
@@ -260,23 +260,27 @@ WinLaunchChild(const PRUnichar *exePath,
     return FALSE;
   }
 
   STARTUPINFOW si = {0};
   si.cb = sizeof(STARTUPINFOW);
   si.lpDesktop = L"winsta0\\Default";
   PROCESS_INFORMATION pi = {0};
 
+  DWORD creationFlags = 0;
+#ifdef DEBUG
+  creationFlags |= CREATE_NEW_CONSOLE;
+#endif
   if (userToken == NULL) {
     ok = CreateProcessW(exePath,
                         cl,
                         NULL,  // no special security attributes
                         NULL,  // no special thread attributes
                         FALSE, // don't inherit filehandles
-                        0,     // No special process creation flags
+                        creationFlags,
                         NULL,  // inherit my environment
                         NULL,  // use my current directory
                         &si,
                         &pi);
   } else {
     // Create an environment block for the process we're about to start using
     // the user's token.
     LPVOID environmentBlock = NULL;