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 342252 27ed205d7628f2a653208c5dd0d9047725a1250c
parent 342251 eee8f9b6791deff3db5fb7a3423ed04fdc4e6725
child 342253 45e8e39788c822f4c4ade9352a380f72e3e4b9f1
push id31346
push userkwierso@gmail.com
push dateFri, 10 Feb 2017 22:33:24 +0000
treeherdermozilla-central@7b9d9e4a82a6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrstrong
bugs1335916
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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);
     }