Bug 1335916 - Make sure the update driver only calls SetupMacCommandLine from the main thread. r?rstrong draft
authorMatt Howell <mhowell@mozilla.com>
Thu, 02 Feb 2017 15:29:32 -0800
changeset 469857 a24fcaed937443a5fe0c6b527d9fa8bfdb560d3c
parent 469123 f3d187bd0733b1182dffc97b5dfe623e18f92a44
child 544337 68b2ef764000db8170e9c787bdf4c2ff2e8d61f5
push id43879
push userbmo:mhowell@mozilla.com
push dateThu, 02 Feb 2017 23:30:29 +0000
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);
     }