Bug 1335916 - Make sure the update driver only calls SetupMacCommandLine from the main thread. r=rstrong
authorMatt Howell <mhowell@mozilla.com>
Thu, 02 Feb 2017 15:29:32 -0800
changeset 371279 27ed205d7628f2a653208c5dd0d9047725a1250c
parent 371278 eee8f9b6791deff3db5fb7a3423ed04fdc4e6725
child 371280 45e8e39788c822f4c4ade9352a380f72e3e4b9f1
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrstrong
bugs1335916
milestone54.0a1
Bug 1335916 - Make sure the update driver only calls SetupMacCommandLine from the main thread. r=rstrong MozReview-Commit-ID: 9nOgB6z8ooE
toolkit/xre/nsUpdateDriver.cpp
--- a/toolkit/xre/nsUpdateDriver.cpp
+++ b/toolkit/xre/nsUpdateDriver.cpp
@@ -27,16 +27,17 @@
 #include "nsPrintfCString.h"
 #include "mozilla/DebugOnly.h"
 
 #ifdef XP_MACOSX
 #include "nsILocalFileMac.h"
 #include "nsCommandLineServiceMac.h"
 #include "MacLaunchHelper.h"
 #include "updaterfileutils_osx.h"
+#include "mozilla/Monitor.h"
 #endif
 
 #if defined(XP_WIN)
 # include <direct.h>
 # include <process.h>
 # include <windows.h>
 # include <shlwapi.h>
 # include "nsWindowsHelpers.h"
@@ -86,16 +87,43 @@ static const char kAppUpdaterIOPrioClass
 static const char kAppUpdaterIOPrioLevel[] = "app.update.updater.ioprio.level";
 
 static const int  kAppUpdaterPrioDefault        = 19;     // -20..19 where 19 = lowest priority
 static const int  kAppUpdaterOomScoreAdjDefault = -1000;  // -1000 = Never kill
 static const int  kAppUpdaterIOPrioClassDefault = IOPRIO_CLASS_IDLE;
 static const int  kAppUpdaterIOPrioLevelDefault = 0;      // Doesn't matter for CLASS IDLE
 #endif
 
+#ifdef XP_MACOSX
+static void
+UpdateDriverSetupMacCommandLine(int& argc, char**& argv, bool restart)
+{
+  if (NS_IsMainThread()) {
+    CommandLineServiceMac::SetupMacCommandLine(argc, argv, restart);
+    return;
+  }
+  // Bug 1335916: SetupMacCommandLine calls a CoreFoundation function that
+  // asserts that it was called from the main thread, so if we are not the main
+  // thread, we have to dispatch that call to there. But we also have to get the
+  // result from it, so we can't just dispatch and return, we have to wait
+  // until the dispatched operation actually completes. So we also set up a
+  // monitor to signal us when that happens, and block until then.
+  Monitor monitor("nsUpdateDriver SetupMacCommandLine");
+
+  NS_DispatchToMainThread(
+    NS_NewRunnableFunction([&argc, &argv, restart, &monitor]() -> void
+    {
+      CommandLineServiceMac::SetupMacCommandLine(argc, argv, restart);
+      MonitorAutoLock(monitor).Notify();
+    }));
+
+  MonitorAutoLock(monitor).Wait();
+}
+#endif
+
 static nsresult
 GetCurrentWorkingDir(char *buf, size_t size)
 {
   // Cannot use NS_GetSpecialDirectory because XPCOM is not yet initialized.
   // This code is duplicated from xpcom/io/SpecialSystemDirectory.cpp:
 
 #if defined(XP_WIN)
   wchar_t wpath[MAX_PATH];
@@ -633,17 +661,17 @@ SwitchToUpdatedApp(nsIFile *greDir, nsIF
   exit(execv(updaterPath.get(), argv));
 #elif defined(XP_WIN)
   // Switch the application using updater.exe
   if (!WinLaunchChild(updaterPathW.get(), argc, argv)) {
     return;
   }
   _exit(0);
 #elif defined(XP_MACOSX)
-  CommandLineServiceMac::SetupMacCommandLine(argc, argv, true);
+  UpdateDriverSetupMacCommandLine(argc, argv, true);
   LaunchChildMac(argc, argv);
   exit(0);
 #else
   PR_CreateProcessDetached(updaterPath.get(), argv, nullptr, nullptr);
   exit(0);
 #endif
 }
 
@@ -951,17 +979,17 @@ ApplyUpdate(nsIFile *greDir, nsIFile *up
     return;
   }
 
   if (restart) {
     // We are going to process an update so we should exit now
     _exit(0);
   }
 #elif defined(XP_MACOSX)
-  CommandLineServiceMac::SetupMacCommandLine(argc, argv, restart);
+  UpdateDriverSetupMacCommandLine(argc, argv, restart);
   // We need to detect whether elevation is required for this update. This can
   // occur when an admin user installs the application, but another admin
   // user attempts to update (see bug 394984).
   if (restart && !IsRecursivelyWritable(installDirPath.get())) {
     if (!LaunchElevatedUpdate(argc, argv, outpid)) {
       LOG(("Failed to launch elevated update!"));
       exit(1);
     }