Bug 711792 - Speed up and simplify maintenance service update command processing. r=rstrong.
authorBrian R. Bondy <netzen@gmail.com>
Wed, 04 Jan 2012 23:19:17 -0500
changeset 85008 dabd7c594ea9eb7b0adf994e19edf1ae9a21f55a
parent 85007 9d47544d1abadbe1f99615eb22b070abdd66abc8
child 85009 8c8f118947645ee7a3ead3ea71aa2a6f0fd598a8
push idunknown
push userunknown
push dateunknown
reviewersrstrong
bugs711792
milestone12.0a1
Bug 711792 - Speed up and simplify maintenance service update command processing. r=rstrong.
browser/installer/windows/nsis/maintenanceservice_installer.nsi
toolkit/components/maintenanceservice/maintenanceservice.cpp
toolkit/components/maintenanceservice/maintenanceservice.h
toolkit/components/maintenanceservice/workmonitor.cpp
toolkit/components/maintenanceservice/workmonitor.h
toolkit/mozapps/update/common/updatehelper.cpp
toolkit/mozapps/update/common/updatehelper.h
toolkit/mozapps/update/updater/updater.cpp
--- a/browser/installer/windows/nsis/maintenanceservice_installer.nsi
+++ b/browser/installer/windows/nsis/maintenanceservice_installer.nsi
@@ -232,21 +232,16 @@ Section "MaintenanceService"
   WriteRegDWORD HKLM "Software\Mozilla\MaintenanceService" "Installed" 1
 
   ; Included here for debug purposes only.  
   ; These keys are used to bypass the installation dir is a valid installation
   ; check from the service so that tests can be run.
   ; WriteRegStr HKLM "${FallbackKey}\0" "name" "Mozilla Corporation"
   ; WriteRegStr HKLM "${FallbackKey}\0" "issuer" "Thawte Code Signing CA - G2"
   SetRegView lastused
-
-  # The Mozilla/updates directory will have an inherited permission
-  # which allows any user to write to it.  Work items are written there.
-  SetShellVarContext all
-  CreateDirectory "$APPDATA\Mozilla\updates"
 SectionEnd
 
 ; By renaming before deleting we improve things slightly in case
 ; there is a file in use error. In this case a new install can happen.
 Function un.RenameDelete
   Pop $9
   ; If the .moz-delete file already exists previously, delete it
   ; If it doesn't exist, the call is ignored.
--- a/toolkit/components/maintenanceservice/maintenanceservice.cpp
+++ b/toolkit/components/maintenanceservice/maintenanceservice.cpp
@@ -142,19 +142,21 @@ wmain(int argc, WCHAR **argv)
 }
 
 /**
  * Wrapper callback for the monitoring thread.
  *
  * @param param Unused thread callback parameter
  */
 DWORD
-WINAPI StartMonitoringThreadProc(LPVOID param) 
+WINAPI StartMaintenanceServiceThread(LPVOID param) 
 {
-  StartDirectoryChangeMonitor();
+  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.
@@ -268,62 +270,41 @@ SvcMain(DWORD dwArgc, LPWSTR *lpszArgv)
   // Perform service-specific initialization and work.
   SvcInit(dwArgc, lpszArgv);
 }
 
 /**
  * Service initialization.
  */
 void
-SvcInit(DWORD dwArgc, LPWSTR *lpszArgv)
+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) {
     ReportSvcStatus(SERVICE_STOPPED, 1, 0);
     return;
   }
 
+  ThreadData *threadData = new ThreadData();
+  threadData->argc = argc;
+  threadData->argv = argv;
+
   DWORD threadID;
-  HANDLE thread = CreateThread(NULL, 0, StartMonitoringThreadProc, 0, 
-                               0, &threadID);
+  HANDLE thread = CreateThread(NULL, 0, StartMaintenanceServiceThread, 
+                               threadData, 0, &threadID);
 
   // Report running status when initialization is complete.
   ReportSvcStatus(SERVICE_RUNNING, NO_ERROR, 0);
 
   // Perform work until service stops.
   for(;;) {
     // Check whether to stop the service.
     WaitForSingleObject(ghSvcStopEvent, INFINITE);
-
-    WCHAR stopFilePath[MAX_PATH +1];
-    if (!GetUpdateDirectoryPath(stopFilePath)) {
-      LOG(("Could not obtain update directory path, terminating thread "
-           "forcefully.\n"));
-      TerminateThread(thread, 1);
-    }
-
-    // The stop file is to wake up the synchronous call for watching directory
-    // changes. Directory watching will only actually be stopped if 
-    // gServiceStopping is also set to TRUE.
-    gServiceStopping = TRUE;
-    if (!PathAppendSafe(stopFilePath, L"stop")) {
-      TerminateThread(thread, 2);
-    }
-    HANDLE stopFile = CreateFile(stopFilePath, GENERIC_READ, 0, 
-                                 NULL, CREATE_ALWAYS, 0, NULL);
-    if (stopFile == INVALID_HANDLE_VALUE) {
-      LOG(("Could not create stop file, terminating thread forcefully.\n"));
-      TerminateThread(thread, 3);
-    } else {
-      CloseHandle(stopFile);
-      DeleteFile(stopFilePath);
-    }
-
     ReportSvcStatus(SERVICE_STOPPED, NO_ERROR, 0);
     return;
   }
 }
 
 /**
  * Sets the current service status and reports it to the SCM.
  *  
--- a/toolkit/components/maintenanceservice/maintenanceservice.h
+++ b/toolkit/components/maintenanceservice/maintenanceservice.h
@@ -36,8 +36,12 @@
  * ***** 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/workmonitor.cpp
+++ b/toolkit/components/maintenanceservice/workmonitor.cpp
@@ -39,29 +39,29 @@
 #include <shlwapi.h>
 #include <wtsapi32.h>
 #include <userenv.h>
 #include <shellapi.h>
 
 #pragma comment(lib, "wtsapi32.lib")
 #pragma comment(lib, "userenv.lib")
 #pragma comment(lib, "shlwapi.lib")
+#pragma comment(lib, "ole32.lib")
+#pragma comment(lib, "rpcrt4.lib")
 
 #include "nsWindowsHelpers.h"
 #include "nsAutoPtr.h"
 
 #include "workmonitor.h"
 #include "serviceinstall.h"
 #include "servicebase.h"
 #include "registrycertificates.h"
 #include "uachelper.h"
 #include "updatehelper.h"
 
-extern BOOL gServiceStopping;
-
 // 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);
@@ -72,45 +72,42 @@ const int SERVICE_UPDATER_COULD_NOT_BE_S
 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;
 
 /**
  * Runs an update process as the service using the SYSTEM account.
  *
- * @param  updaterPath    The path to the update process to start.
- * @param  workingDir     The working directory to execute the update process in
- * @param  cmdLine        The command line parameters to pass to updater.exe
+ * @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.
  * @return TRUE if the update process was run had a return code of 0.
  */
 BOOL
-StartUpdateProcess(LPCWSTR updaterPath, 
-                   LPCWSTR workingDir, 
-                   int argcTmp,
-                   LPWSTR *argvTmp,
+StartUpdateProcess(int argc,
+                   LPWSTR *argv,
                    BOOL &processStarted)
 {
+  LOG(("Starting update process as the service in session 0.\n"));
   STARTUPINFO si = {0};
   si.cb = sizeof(STARTUPINFO);
   si.lpDesktop = L"winsta0\\Default";
   PROCESS_INFORMATION pi = {0};
 
-  LOG(("Starting update process as the service in session 0.\n"));
-
   // The updater command line is of the form:
   // updater.exe update-dir apply [wait-pid [callback-dir callback-path args]]
-  LPWSTR cmdLine = MakeCommandLine(argcTmp, argvTmp);
+  LPWSTR cmdLine = MakeCommandLine(argc, argv);
 
   // If we're about to start the update process from session 0,
   // then we should not show a GUI.  This only really needs to be done
   // on Vista and higher, but it's better to keep everything consistent
   // across all OS if it's of no harm.
-  if (argcTmp >= 2 ) {
+  if (argc >= 2 ) {
     // Setting the desktop to blank will ensure no GUI is displayed
     si.lpDesktop = L"";
     si.dwFlags |= STARTF_USESHOWWINDOW;
     si.wShowWindow = SW_HIDE;
   }
 
   // We move the updater.ini file out of the way because we will handle 
   // executing PostUpdate through the service.  We handle PostUpdate from
@@ -118,18 +115,18 @@ StartUpdateProcess(LPCWSTR updaterPath,
   // can't run in session 0 which we run updater.exe in.
   // Once we are done running updater.exe we rename updater.ini back so
   // 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, argvTmp[0], L"updater.ini") &&
-      PathGetSiblingFilePath(updaterINITemp, argvTmp[0], L"updater.tmp")) {
+  if (PathGetSiblingFilePath(updaterINI, argv[0], L"updater.ini") &&
+      PathGetSiblingFilePath(updaterINITemp, argv[0], L"updater.tmp")) {
     selfHandlePostUpdate = MoveFileEx(updaterINI, updaterINITemp, 
                                       MOVEFILE_REPLACE_EXISTING);
   }
 
   // Create an environment block for the updater.exe process we're about to 
   // start.  Indicate that MOZ_USING_SERVICE is set 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.
@@ -138,22 +135,22 @@ StartUpdateProcess(LPCWSTR updaterPath,
   _wputenv(envVarString);
   LPVOID environmentBlock = NULL;
   if (!CreateEnvironmentBlock(&environmentBlock, NULL, TRUE)) {
     LOG(("Could not create an environment block, setting it to NULL.\n"));
     environmentBlock = NULL;
   }
   // Empty value on _wputenv is how you remove an env variable in Windows
   _wputenv(L"MOZ_USING_SERVICE=");
-  processStarted = CreateProcessW(updaterPath, cmdLine, 
+  processStarted = CreateProcessW(argv[0], cmdLine, 
                                   NULL, NULL, FALSE, 
                                   CREATE_DEFAULT_ERROR_MODE | 
                                   CREATE_UNICODE_ENVIRONMENT, 
                                   environmentBlock, 
-                                  workingDir, &si, &pi);
+                                  NULL, &si, &pi);
   if (environmentBlock) {
     DestroyEnvironmentBlock(environmentBlock);
   }
   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);
@@ -173,29 +170,29 @@ StartUpdateProcess(LPCWSTR updaterPath,
       }
     }
     CloseHandle(pi.hProcess);
     CloseHandle(pi.hThread);
   } else {
     DWORD lastError = GetLastError();
     LOG(("Could not create process as current user, "
          "updaterPath: %ls; cmdLine: %l.  (%d)\n", 
-         updaterPath, cmdLine, lastError));
+         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);
 
     // Only run the PostUpdate if the update was successful
-    if (updateWasSuccessful && argcTmp > 2) {
-      LPCWSTR installationDir = argvTmp[2];
-      LPCWSTR updateInfoDir = argvTmp[1];
+    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 
       // callback app path to figure out where to apply to.
       // The PostUpdate process with user only access will be done inside
       // the unelevated updater.exe after the update process is complete
       // from the service.  We don't know here which session to start
       // the user PostUpdate process from.
@@ -206,189 +203,80 @@ StartUpdateProcess(LPCWSTR updaterPath,
     }
   }
 
   free(cmdLine);
   return updateWasSuccessful;
 }
 
 /**
- * Processes a work item (file by the name of .mz) and executes
- * the command within.
- * The only supported command at this time is running an update.
+ * Processes a software update command
  *
- * @param  monitoringBasePath The base path that is being monitored.
- * @param  notifyInfo         the notifyInfo struct for the changes.
- * @return TRUE if we want the service to stop.
+ * @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
-ProcessWorkItem(LPCWSTR monitoringBasePath, 
-                FILE_NOTIFY_INFORMATION &notifyInfo)
+ProessSoftwareUpdateCommand(DWORD argc, LPWSTR *argv)
 {
-  int filenameLength = notifyInfo.FileNameLength / 
-                       sizeof(notifyInfo.FileName[0]); 
-  notifyInfo.FileName[filenameLength] = L'\0';
+  BOOL result = TRUE;
+  if (argc < 3) {
+    LOG(("Not enough command line parameters specified. "
+         "Updating update.status.\n"));
 
-  // When the file is ready for processing it will be renamed 
-  // to have a .mz extension
-  BOOL haveWorkItem = notifyInfo.Action == FILE_ACTION_RENAMED_NEW_NAME && 
-                      notifyInfo.FileNameLength > 3 && 
-                      notifyInfo.FileName[filenameLength - 3] == L'.' &&
-                      notifyInfo.FileName[filenameLength - 2] == L'm' &&
-                      notifyInfo.FileName[filenameLength - 1] == L'z';
-  if (!haveWorkItem) {
-    // We don't have a work item, keep looking
+    // We can only update update.status if argv[1] exists.  argv[1] is
+    // the directory where the update.status file exists.
+    if (argc > 1 || 
+        !WriteStatusFailure(argv[1], 
+                            SERVICE_NOT_ENOUGH_COMMAND_LINE_ARGS)) {
+      LOG(("Could not write update.status service update failure."
+           "Last error: %d\n", GetLastError()));
+    }
     return FALSE;
   }
 
-  // Indicate that the service is busy and shouldn't be used by anyone else
-  // by opening or creating a named event.  Programs should check if this 
-  // event exists before writing a work item out.  It should already be
-  // created by updater.exe so CreateEventW will lead to an open named event.
-  nsAutoHandle serviceRunningEvent(CreateEventW(NULL, TRUE, 
-                                                FALSE, SERVICE_EVENT_NAME));
-
-  LOG(("Processing new command meta file: %ls\n", notifyInfo.FileName));
-  WCHAR fullMetaUpdateFilePath[MAX_PATH + 1];
-  wcscpy(fullMetaUpdateFilePath, monitoringBasePath);
-
-  // We only support file paths in monitoring directories that are MAX_PATH
-  // chars or less.
-  if (!PathAppendSafe(fullMetaUpdateFilePath, notifyInfo.FileName)) {
-    SetEvent(serviceRunningEvent);
-    return TRUE;
-  }
-
-  nsAutoHandle metaUpdateFile(CreateFile(fullMetaUpdateFilePath, 
-                                         GENERIC_READ, 
-                                         FILE_SHARE_READ | 
-                                         FILE_SHARE_WRITE | 
-                                         FILE_SHARE_DELETE, 
-                                         NULL, 
-                                         OPEN_EXISTING,
-                                         0, NULL));
-  if (metaUpdateFile == INVALID_HANDLE_VALUE) {
-    LOG(("Could not open command meta file: %ls\n", notifyInfo.FileName));
-    SetEvent(serviceRunningEvent);
-    return TRUE;
-  }
-
-  DWORD fileSize = GetFileSize(metaUpdateFile, NULL);
-  DWORD commandID = 0;
-  // The file should be in wide characters so if it's of odd size it's
-  // an invalid file.
-  const int kSanityCheckFileSize = 1024 * 64;
-  if (fileSize == INVALID_FILE_SIZE || 
-      (fileSize %2) != 0 ||
-      fileSize > kSanityCheckFileSize ||
-      fileSize < sizeof(DWORD)) {
-    LOG(("Could not obtain file size or an improper file size was encountered "
-         "for command meta file: %ls\n", 
-        notifyInfo.FileName));
-    SetEvent(serviceRunningEvent);
-    return TRUE;
-  }
-
-  // The first 4 bytes are for the command ID.
-  // Currently only command ID 1 which is for updates is supported.
-  DWORD commandIDCount;
-  BOOL result = ReadFile(metaUpdateFile, &commandID, 
-                         sizeof(DWORD), &commandIDCount, NULL);
-  fileSize -= sizeof(DWORD);
-
-  // The next MAX_PATH wchar's are for the app to start
-  WCHAR updaterPath[MAX_PATH + 1];
-  ZeroMemory(updaterPath, sizeof(updaterPath));
-  DWORD updaterPathCount;
-  result |= ReadFile(metaUpdateFile, updaterPath, 
-                     MAX_PATH * sizeof(WCHAR), &updaterPathCount, NULL);
-  fileSize -= updaterPathCount;
-
-  // The next MAX_PATH wchar's are for the app to start
-  WCHAR workingDirectory[MAX_PATH + 1];
-  ZeroMemory(workingDirectory, sizeof(workingDirectory));
-  DWORD workingDirectoryCount;
-  result |= ReadFile(metaUpdateFile, workingDirectory, 
-                     MAX_PATH * sizeof(WCHAR), &workingDirectoryCount, NULL);
-  fileSize -= workingDirectoryCount;
-
-  // + 2 for wide char termination
-  nsAutoArrayPtr<char> cmdlineBuffer = new char[fileSize + 2];
-  DWORD cmdLineBufferRead;
-  result |= ReadFile(metaUpdateFile, cmdlineBuffer, 
-                     fileSize, &cmdLineBufferRead, NULL);
-  fileSize -= cmdLineBufferRead;
-
-  // We have all the info we need from the work item file, get rid of it.
-  metaUpdateFile.reset();
-  if (!DeleteFileW(fullMetaUpdateFilePath)) {
-    LOG(("Could not delete work item file: %ls. (%d)\n", 
-         fullMetaUpdateFilePath, GetLastError()));
-  }
-
-  if (!result ||
-      commandID != 1 ||
-      commandIDCount != sizeof(DWORD) ||
-      updaterPathCount != MAX_PATH * sizeof(WCHAR) ||
-      workingDirectoryCount != MAX_PATH * sizeof(WCHAR) ||
-      fileSize != 0) {
-    LOG(("Could not read command data for command meta file: %ls\n", 
-         notifyInfo.FileName));
-    SetEvent(serviceRunningEvent);
-    return TRUE;
-  }
-  cmdlineBuffer[cmdLineBufferRead] = '\0';
-  cmdlineBuffer[cmdLineBufferRead + 1] = '\0';
-  WCHAR *cmdlineBufferWide = reinterpret_cast<WCHAR*>(cmdlineBuffer.get());
-  LOG(("An update command was detected and is being processed for command meta "
-       "file: %ls\n", notifyInfo.FileName));
-
-  int argcTmp = 0;
-  LPWSTR* argvTmp = CommandLineToArgvW(cmdlineBufferWide, &argcTmp);
-
   // Verify that the updater.exe that we are executing is the same
   // as the one in the installation directory which we are updating.
-  // The installation dir that we are installing to is argvTmp[2].
+  // The installation dir that we are installing to is argv[2].
   WCHAR installDirUpdater[MAX_PATH + 1];
-  wcsncpy(installDirUpdater, argvTmp[2], MAX_PATH);
+  wcsncpy(installDirUpdater, argv[2], MAX_PATH);
   if (!PathAppendSafe(installDirUpdater, L"updater.exe")) {
     LOG(("Install directory updater could not be determined.\n"));
     result = FALSE;
   }
 
   BOOL updaterIsCorrect;
-  if (result && !VerifySameFiles(updaterPath, installDirUpdater, 
+  if (result && !VerifySameFiles(argv[0], installDirUpdater, 
                                  updaterIsCorrect)) {
     LOG(("Error checking if the updaters are the same.\n"
-         "Path 1: %ls\nPath 2: %ls\n", updaterPath, installDirUpdater));
+         "Path 1: %ls\nPath 2: %ls\n", argv[0], installDirUpdater));
     result = FALSE;
   }
 
   if (result && !updaterIsCorrect) {
     LOG(("The updaters do not match, udpater will not run.\n")); 
     result = FALSE;
   }
 
   if (result) {
     LOG(("updater.exe was compared successfully to the installation directory"
          " updater.exe.\n"));
   } else {
-    SetEvent(serviceRunningEvent);
-    if (argcTmp < 2 || 
-        !WriteStatusFailure(argvTmp[1], 
+    if (!WriteStatusFailure(argv[1], 
                             SERVICE_UPDATER_COMPARE_ERROR)) {
       LOG(("Could not write update.status updater compare failure.\n"));
     }
-    return TRUE;
+    return FALSE;
   }
 
   // Check to make sure the udpater.exe module has the unique updater identity.
   // This is a security measure to make sure that the signed executable that
   // we will run is actually an updater.
-  HMODULE updaterModule = LoadLibrary(updaterPath);
+  HMODULE updaterModule = LoadLibrary(argv[0]);
   if (!updaterModule) {
     LOG(("updater.exe module could not be loaded. (%d)\n", GetLastError()));
     result = FALSE;
   } else {
     char updaterIdentity[64];
     if (!LoadStringA(updaterModule, IDS_UPDATER_IDENTITY, 
                      updaterIdentity, sizeof(updaterIdentity))) {
       LOG(("The updater.exe application does not contain the Mozilla"
@@ -402,161 +290,117 @@ ProcessWorkItem(LPCWSTR monitoringBasePa
     }
     FreeLibrary(updaterModule);
   }
 
   if (result) {
     LOG(("The updater.exe application contains the Mozilla"
           " updater identity.\n"));
   } else {
-    SetEvent(serviceRunningEvent);
-    if (argcTmp < 2 || 
-        !WriteStatusFailure(argvTmp[1], 
+    if (!WriteStatusFailure(argv[1], 
                             SERVICE_UPDATER_IDENTITY_ERROR)) {
       LOG(("Could not write update.status no updater identity.\n"));
     }
     return TRUE;
   }
 
   // Check for updater.exe sign problems
   BOOL updaterSignProblem = FALSE;
 #ifndef DISABLE_UPDATER_AUTHENTICODE_CHECK
-  if (argcTmp > 2) {
-    updaterSignProblem = !DoesBinaryMatchAllowedCertificates(argvTmp[2], 
-                                                             updaterPath);
-  }
+  updaterSignProblem = !DoesBinaryMatchAllowedCertificates(argv[2],
+                                                           argv[0]);
 #endif
 
-  // In order to proceed with the update we need at least 3 command line
-  // parameters and no sign problems.
-  if (argcTmp > 2 && !updaterSignProblem) {
+  // Only proceed with the update if we have no signing problems
+  if (!updaterSignProblem) {
     BOOL updateProcessWasStarted = FALSE;
-    if (StartUpdateProcess(updaterPath, workingDirectory, 
-                           argcTmp, argvTmp,
+    if (StartUpdateProcess(argc, argv,
                            updateProcessWasStarted)) {
       LOG(("updater.exe was launched and run successfully!\n"));
-      StartServiceUpdate(argcTmp, argvTmp);
+      StartServiceUpdate(argc, argv);
     } else {
+      result = FALSE;
       LOG(("Error running update process. Updating update.status"
            " Last error: %d\n", GetLastError()));
 
       // If the update process was started, then updater.exe is responsible for
       // setting the failure code.  If it could not be started then we do the 
       // work.  We set an error instead of directly setting status pending 
       // so that the app.update.service.errors pref can be updated when 
       // the callback app restarts.
       if (!updateProcessWasStarted) {
-        if (!WriteStatusFailure(argvTmp[1], 
+        if (!WriteStatusFailure(argv[1], 
                                 SERVICE_UPDATER_COULD_NOT_BE_STARTED)) {
           LOG(("Could not write update.status service update failure."
                "Last error: %d\n", GetLastError()));
         }
       }
     }
-  } else if (argcTmp <= 2) {
-    LOG(("Not enough command line parameters specified. "
-         "Updating update.status.\n"));
-
-    // We can only update update.status if argvTmp[1] exists.  argvTmp[1] is
-    // the directory where the update.status file exists.
-    if (argcTmp != 2 || 
-        !WriteStatusFailure(argvTmp[1], 
-                            SERVICE_NOT_ENOUGH_COMMAND_LINE_ARGS)) {
-      LOG(("Could not write update.status service update failure."
-           "Last error: %d\n", GetLastError()));
-    }
   } else {
+    result = FALSE;
     LOG(("Could not start process due to certificate check error on "
          "updater.exe. Updating update.status.  Last error: %d\n", GetLastError()));
 
     // When there is a certificate check error on the updater.exe application,
     // we want to write out the error.
-    if (!WriteStatusFailure(argvTmp[1], 
+    if (!WriteStatusFailure(argv[1], 
                             SERVICE_UPDATER_SIGN_ERROR)) {
       LOG(("Could not write pending state to update.status.  (%d)\n", 
            GetLastError()));
     }
   }
-  LocalFree(argvTmp);
-  SetEvent(serviceRunningEvent);
-
-  // We processed a work item, whether or not it was successful.
-  return TRUE;
+  LocalFree(argv);
+  return result;
 }
 
 /**
- * Starts monitoring the update directory for work items.
+ * Executes a service command.
  *
- * @return FALSE if there was an error starting directory monitoring.
+ * @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
-StartDirectoryChangeMonitor() 
+ExecuteServiceCommand(int argc, LPWSTR *argv) 
 {
-  LOG(("Starting directory change monitor...\n"));
+  // Indicate that the service is busy and shouldn't be used by anyone else
+  // by opening or creating a named event.  Programs should check if this 
+  // event exists before trying to start the service.
+  nsAutoHandle serviceRunningEvent(CreateEventW(NULL, TRUE, 
+                                                FALSE, SERVICE_EVENT_NAME));
 
-  // Init the update directory path and ensure it exists.
-  // Example: C:\programData\Mozilla\Firefox\updates\[channel]
-  // The channel is not included here as we want to monitor the base directory
-  WCHAR updateData[MAX_PATH + 1];
-  if (!GetUpdateDirectoryPath(updateData)) {
-    LOG(("Could not obtain update directory path\n"));
-    return FALSE;
-  }
-
-  // Obtain a directory handle, FILE_FLAG_BACKUP_SEMANTICS is needed for this.
-  nsAutoHandle directoryHandle(CreateFile(updateData, 
-                                          FILE_LIST_DIRECTORY, 
-                                          FILE_SHARE_READ | FILE_SHARE_WRITE, 
-                                          NULL, 
-                                          OPEN_EXISTING,
-                                          FILE_FLAG_BACKUP_SEMANTICS, 
-                                          NULL));
-  if (directoryHandle == INVALID_HANDLE_VALUE) {
-    LOG(("Could not obtain directory handle to monitor\n"));
+  if (argc < 3) {
+    LOG(("Not enough command line arguments to execute a service command\n"));
+    SetEvent(serviceRunningEvent);
+    StopService();
     return FALSE;
   }
 
-  // Allocate a buffer that is big enough to hold 128 changes
-  // Note that FILE_NOTIFY_INFORMATION is a variable length struct
-  // so that's why we don't create a simple array directly.
-  char fileNotifyInfoBuffer[(sizeof(FILE_NOTIFY_INFORMATION) + 
-                            MAX_PATH) * 128];
-  ZeroMemory(&fileNotifyInfoBuffer, sizeof(fileNotifyInfoBuffer));
-  
-  DWORD bytesReturned;
-  // Listen for directory changes to the update directory
-  while (ReadDirectoryChangesW(directoryHandle, 
-                               fileNotifyInfoBuffer, 
-                               sizeof(fileNotifyInfoBuffer), 
-                               TRUE, FILE_NOTIFY_CHANGE_FILE_NAME, 
-                               &bytesReturned, NULL, NULL)) {
-    char *fileNotifyInfoBufferLocation = fileNotifyInfoBuffer;
+  // 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);
+  if (SUCCEEDED(hr)) {
+    UuidToString(&guid, &guidString);
+  }
+  LOG(("Executing service command %ls, ID: %ls\n", 
+       argv[2], reinterpret_cast<LPCWSTR>(guidString)));
+  RpcStringFree(&guidString);
 
-    // Make sure we have at least one entry to process
-    while (bytesReturned/sizeof(FILE_NOTIFY_INFORMATION) > 0) {
-      // Point to the current directory info which is changed
-      FILE_NOTIFY_INFORMATION &notifyInfo = 
-        *((FILE_NOTIFY_INFORMATION*)fileNotifyInfoBufferLocation);
-      fileNotifyInfoBufferLocation += notifyInfo .NextEntryOffset;
-      bytesReturned -= notifyInfo .NextEntryOffset;
-      if (!wcscmp(notifyInfo.FileName, L"stop") && gServiceStopping) {
-        LOG(("A stop command was issued.\n"));
-        return TRUE;
-      }
-
-      BOOL processedWorkItem = ProcessWorkItem(updateData, notifyInfo);
-      if (processedWorkItem) {
-        // We don't return here because during stop we will write out a stop 
-        // file which will stop monitoring.
-        StopService();
-        break;
-      }
-
-      // NextEntryOffset will be 0 if there are no more items to monitor,
-      // and we're ready to make another call to ReadDirectoryChangesW.
-      if (0 == notifyInfo.NextEntryOffset) {
-        break;
-      }
-    }
+  BOOL result = FALSE;
+  if (!lstrcmpi(argv[2], L"software-update")) {
+    result = ProessSoftwareUpdateCommand(argc - 3, argv + 3);
+    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(serviceRunningEvent);
+  StopService();
   return TRUE;
 }
--- a/toolkit/components/maintenanceservice/workmonitor.h
+++ b/toolkit/components/maintenanceservice/workmonitor.h
@@ -30,12 +30,10 @@
  * 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 ***** */
 
-#define STOP_CMD L"stop"
-
-BOOL StartDirectoryChangeMonitor();
+BOOL ExecuteServiceCommand(int argc, LPWSTR *argv);
 BOOL GetUpdateDirectoryPath(LPWSTR);
--- a/toolkit/mozapps/update/common/updatehelper.cpp
+++ b/toolkit/mozapps/update/common/updatehelper.cpp
@@ -43,44 +43,16 @@
 #include <shlwapi.h>
 #pragma comment(lib, "shlwapi.lib") 
 
 WCHAR*
 MakeCommandLine(int argc, WCHAR **argv);
 BOOL PathAppendSafe(LPWSTR base, LPCWSTR extra);
 
 /**
- * Obtains the directory path to store work item files.
- * 
- * @param  path The buffer of size MAX_PATH to store the update directory to
- * @return TRUE if the path was obtained successfully.
-*/
-BOOL
-GetUpdateDirectoryPath(LPWSTR path) 
-{
-  HRESULT hr = SHGetFolderPathW(NULL, CSIDL_COMMON_APPDATA, NULL, 
-                                SHGFP_TYPE_CURRENT, path);
-  if (FAILED(hr)) {
-    return FALSE;
-  }
-  if (!PathAppendSafe(path, L"Mozilla")) {
-    return FALSE;
-  }
-  // The directory should already be created from the installer, but
-  // just to be safe in case someone deletes.
-  CreateDirectoryW(path, NULL);
-
-  if (!PathAppendSafe(path, L"updates")) {
-    return FALSE;
-  }
-  CreateDirectoryW(path, NULL);
-  return TRUE;
-}
-
-/**
  * 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
  * @return TRUE if successful
  */
 BOOL
@@ -292,22 +264,24 @@ StartServiceUpdate(int argc, LPWSTR *arg
     CloseHandle(pi.hProcess);
     CloseHandle(pi.hThread);
   }
   return svcUpdateProcessStarted;
 }
 
 
 /**
- * Determines if the maintenance service is running or not.
+ * Executes a maintenance service command
  * 
- * @return TRUE if the maintenance service is running.
+ * @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.
 */
 BOOL 
-EnsureWindowsServiceRunning() 
+StartServiceCommand(int argc, LPCWSTR* argv) 
 {
   // Get a handle to the SCM database.
   SC_HANDLE serviceManager = OpenSCManager(NULL, NULL, 
                                            SC_MANAGER_CONNECT | 
                                            SC_MANAGER_ENUMERATE_SERVICE);
   if (!serviceManager)  {
     return FALSE;
   }
@@ -326,162 +300,55 @@ EnsureWindowsServiceRunning()
   DWORD bytesNeeded;
   if (!QueryServiceStatusEx(service, SC_STATUS_PROCESS_INFO, (LPBYTE)&ssp,
                             sizeof(SERVICE_STATUS_PROCESS), &bytesNeeded)) {
     CloseServiceHandle(service);
     CloseServiceHandle(serviceManager);
     return FALSE;
   }
 
-  if (ssp.dwCurrentState == SERVICE_STOPPED) {
-    if (!StartService(service, 0, NULL)) {
-      CloseServiceHandle(service);
-      CloseServiceHandle(serviceManager);
-      return FALSE;
-    }
-
-    // Make sure we can get into a started state without waiting too long.
-    // This usually starts instantly but the extra code is just in case it
-    // takes longer.
-    DWORD totalWaitTime = 0;
-    static const int maxWaitTime = 1000 * 5; // Never wait more than 5 seconds
-    while (QueryServiceStatusEx(service, SC_STATUS_PROCESS_INFO, (LPBYTE)&ssp,
-                                sizeof(SERVICE_STATUS_PROCESS), &bytesNeeded)) {
-      if (ssp.dwCurrentState == SERVICE_RUNNING) {
-        break;
-      }
-      
-      if (ssp.dwCurrentState == SERVICE_START_PENDING &&
-          totalWaitTime > maxWaitTime) {
-        // We will probably eventually start, but we can't wait any longer.
-        break;
-      }
-      
-      if (ssp.dwCurrentState != SERVICE_START_PENDING) {
-        CloseServiceHandle(service);
-        CloseServiceHandle(serviceManager);
-        return FALSE;
-      }
-
-      Sleep(ssp.dwWaitHint);
-      // Increment by at least 10 milliseconds to ensure we always make 
-      // progress towards maxWaitTime in case dwWaitHint is 0.
-      totalWaitTime += (ssp.dwWaitHint + 10);
-    }
+  // The service is already in use.
+  if (ssp.dwCurrentState != SERVICE_STOPPED) {
+    CloseServiceHandle(service);
+    CloseServiceHandle(serviceManager);
+    return FALSE;
   }
 
-  CloseServiceHandle(service);
-  CloseServiceHandle(serviceManager);
-  return ssp.dwCurrentState == SERVICE_RUNNING;
+  return StartServiceW(service, argc, argv);
 }
 
 /**
- * Launch a service initiated action with the specified arguments.
+ * 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] is ignored
+ *                 argv[0] must be the path to the updater.exe
  * @return TRUE if successful
  */
 BOOL
-WinLaunchServiceCommand(LPCWSTR exePath, int argc, LPWSTR* argv)
+LaunchServiceSoftwareUpdateCommand(DWORD argc, LPCWSTR* argv)
 {
-  // Ensure the service is running, if not we should try to start it, if it is
-  // not in a running state we cannot execute a service command.
-  if (!EnsureWindowsServiceRunning()) {
-    return FALSE;
-  }
-
-  WCHAR updateData[MAX_PATH + 1];
-  if (!GetUpdateDirectoryPath(updateData)) {
-    return FALSE;
-  }
+  // 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[1] = L"software-update";
 
-  // Get a unique filename
-  WCHAR tempFilePath[MAX_PATH + 1];
-  const int USE_SYSTEM_TIME = 0;
-  if (!GetTempFileNameW(updateData, L"moz", USE_SYSTEM_TIME, tempFilePath)) {
-    return FALSE;
-  }
-  
-  const int FILE_SHARE_NONE = 0;
-  HANDLE updateMetaFile = CreateFileW(tempFilePath, GENERIC_WRITE, 
-                                      FILE_SHARE_NONE, NULL, CREATE_ALWAYS, 
-                                      0, NULL);
-  if (updateMetaFile == INVALID_HANDLE_VALUE) {
-    return FALSE;
-  }
-
-  // Write out the command ID.
-  // Command ID 1 is for an update work item file, which is the only supported
-  // command at this time.
-  DWORD commandID = 1, commandIDWrote;
-  BOOL result = WriteFile(updateMetaFile, &commandID, 
-                          sizeof(DWORD), 
-                          &commandIDWrote, NULL);
-
-  // Write out the command line arguments that are passed to updater.exe
-  // updater.exe's command line arguments look like this normally:
-  // updater.exe update-dir apply [wait-pid [callback-dir callback-path args]]
-  // We pass everything including the callback application and its arguments.
-  // The only reason we pass the callback info though is to lock the exe so it
-  // is not launched during update.
-  LPWSTR commandLineBuffer = MakeCommandLine(argc, argv);
-  if (!commandLineBuffer) {
-    return FALSE;
+  for (int i = 0; i < argc; ++i) {
+    updaterServiceArgv[i + 2] = argv[i];
   }
 
-  WCHAR appBuffer[MAX_PATH + 1];
-  ZeroMemory(appBuffer, sizeof(appBuffer));
-  wcscpy(appBuffer, exePath);
-  DWORD appBufferWrote;
-  result |= WriteFile(updateMetaFile, appBuffer, 
-                      MAX_PATH * sizeof(WCHAR), 
-                      &appBufferWrote, NULL);
-
-  WCHAR workingDirectory[MAX_PATH + 1];
-  ZeroMemory(workingDirectory, sizeof(appBuffer));
-  GetCurrentDirectoryW(sizeof(workingDirectory) / sizeof(workingDirectory[0]), 
-                       workingDirectory);
-  DWORD workingDirectoryWrote;
-  result |= WriteFile(updateMetaFile, workingDirectory, 
-                      MAX_PATH * sizeof(WCHAR), 
-                      &workingDirectoryWrote, NULL);
-
-  DWORD commandLineLength = wcslen(commandLineBuffer) * sizeof(WCHAR);
-  DWORD commandLineWrote;
-  result |= WriteFile(updateMetaFile, commandLineBuffer, 
-                      commandLineLength, 
-                      &commandLineWrote, NULL);
-  free(commandLineBuffer);
-  if (!result ||
-      commandIDWrote != sizeof(DWORD) ||
-      appBufferWrote != MAX_PATH * sizeof(WCHAR) ||
-      workingDirectoryWrote != MAX_PATH * sizeof(WCHAR) ||
-      commandLineWrote != commandLineLength) {
-    CloseHandle(updateMetaFile);
-    DeleteFileW(tempFilePath);
-    return FALSE;
-  }
-
-  // Note we construct the 'service work' meta object with a .tmp extension,
-  // When we want the service to start processing it we simply rename it to
-  // have a .mz extension.  This ensures that the service will never try to
-  // process a partial update work meta file. 
-  CloseHandle(updateMetaFile);
-  WCHAR completedMetaFilePath[MAX_PATH + 1];
-  wcscpy(completedMetaFilePath, tempFilePath);
-
-  // Change the file extension of the temp file path from .tmp to .mz
-  LPWSTR extensionPart = 
-    &(completedMetaFilePath[wcslen(completedMetaFilePath) - 3]);
-  wcscpy(extensionPart, L"mz");
-  return MoveFileExW(tempFilePath, completedMetaFilePath, 
-                     MOVEFILE_REPLACE_EXISTING);
+  // Execute the service command by starting the service with
+  // the passed in arguments.
+  BOOL result = StartServiceCommand(argc + 2, updaterServiceArgv);
+  delete[] updaterServiceArgv;
+  return result;
 }
 
 /**
  * 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
--- a/toolkit/mozapps/update/common/updatehelper.h
+++ b/toolkit/mozapps/update/common/updatehelper.h
@@ -36,12 +36,12 @@
  * ***** 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 WinLaunchServiceCommand(LPCWSTR exePath, int argc, WCHAR **argv);
+BOOL LaunchServiceSoftwareUpdateCommand(DWORD argc, LPCWSTR *argv);
 BOOL WriteStatusFailure(LPCWSTR updateDirPath, int errorCode);
 BOOL WriteStatusPending(LPCWSTR updateDirPath);
 #define SERVICE_EVENT_NAME L"Global\\moz-5b780de9-065b-4341-a04f-ddd94b3723e5"
--- a/toolkit/mozapps/update/updater/updater.cpp
+++ b/toolkit/mozapps/update/updater/updater.cpp
@@ -1753,17 +1753,17 @@ int NS_main(int argc, NS_tchar **argv)
           }
         } else {
           useService = FALSE;
         }
       }
 
       HANDLE serviceInUseEvent = NULL;
       if (useService) {
-        // Make sure the service isn't already busy processing another work item.
+        // Make sure the service isn't already busy processing another command.
         // This event will also be used by the service who will signal it when
         // it is done with the udpate.
         serviceInUseEvent = CreateEventW(NULL, TRUE, 
                                          FALSE, SERVICE_EVENT_NAME);
 
         // Only use the service if we know the event can be created and
         // doesn't already exist.
         if (!serviceInUseEvent) {
@@ -1777,24 +1777,24 @@ 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 = WinLaunchServiceCommand(argv[0], argc, argv);
+        useService = LaunchServiceSoftwareUpdateCommand(argc, (LPCWSTR *)argv);
 
         // The command was launched, so we should wait for the work to be done.
         // The service will set the event we wait on when it is done.
         if (useService) {
           WaitForSingleObject(serviceInUseEvent, INFINITE);
-          CloseHandle(serviceInUseEvent);
         }
+        CloseHandle(serviceInUseEvent);
       }
 
       // 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.
       if (useService) {