Bug 1518639: Move startup locking to the remote service. r=jimm
☠☠ backed out by 3d8dd3615c45 ☠ ☠
authorDave Townsend <dtownsend@oxymoronical.com>
Thu, 31 Jan 2019 12:13:34 -0800
changeset 520625 28404f97bb22b726afee8df04184da81138497a2
parent 520624 5373c5bb9ad5bb7c3af1cae390c3612be51176b5
child 520626 fc466857ab39e3d2371f13ddae553c921fb727d2
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm
bugs1518639
milestone67.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 1518639: Move startup locking to the remote service. r=jimm Makes nsRemoteService responsible for the shared lock for the time between attempting to contact a remote server and starting up our own server. Differential Revision: https://phabricator.services.mozilla.com/D19070
toolkit/components/remote/moz.build
toolkit/components/remote/nsRemoteService.cpp
toolkit/components/remote/nsRemoteService.h
toolkit/xre/nsAppRunner.cpp
--- a/toolkit/components/remote/moz.build
+++ b/toolkit/components/remote/moz.build
@@ -22,9 +22,12 @@ if 'gtk' in CONFIG['MOZ_WIDGET_TOOLKIT']
     if CONFIG['MOZ_ENABLE_DBUS']:
         SOURCES += [
             'nsDBusRemoteClient.cpp',
             'nsDBusRemoteServer.cpp',
         ]
         CXXFLAGS += CONFIG['MOZ_DBUS_GLIB_CFLAGS']
     CXXFLAGS += CONFIG['TK_CFLAGS']
 
+LOCAL_INCLUDES += [
+    '../../profile',
+]
 FINAL_LIBRARY = 'xul'
--- a/toolkit/components/remote/nsRemoteService.cpp
+++ b/toolkit/components/remote/nsRemoteService.cpp
@@ -13,24 +13,70 @@
 #    include "nsDBusRemoteClient.h"
 #  endif
 #endif
 #include "nsRemoteService.h"
 
 #include "nsString.h"
 #include "nsServiceManagerUtils.h"
 #include "mozilla/ModuleUtils.h"
+#include "SpecialSystemDirectory.h"
+
+// Time to wait for the remoting service to start
+#define START_TIMEOUT_SEC 5
+#define START_SLEEP_MSEC 100
 
 using namespace mozilla;
 
 extern int gArgc;
 extern char** gArgv;
 
 NS_IMPL_ISUPPORTS(nsRemoteService, nsIObserver)
 
+void nsRemoteService::LockStartup(nsCString& aProgram, const char* aProfile) {
+  nsCOMPtr<nsIFile> mutexDir;
+  nsresult rv = GetSpecialSystemDirectory(OS_TemporaryDirectory,
+                                          getter_AddRefs(mutexDir));
+  if (NS_SUCCEEDED(rv)) {
+    nsAutoCString mutexPath(aProgram);
+    if (aProfile) {
+      mutexPath.Append(NS_LITERAL_CSTRING("_") + nsDependentCString(aProfile));
+    }
+    mutexDir->AppendNative(mutexPath);
+
+    rv = mutexDir->Create(nsIFile::DIRECTORY_TYPE, 0700);
+    if (NS_SUCCEEDED(rv) || rv == NS_ERROR_FILE_ALREADY_EXISTS) {
+      mRemoteLockDir = mutexDir;
+    }
+  }
+
+  if (mRemoteLockDir) {
+    const mozilla::TimeStamp epoch = mozilla::TimeStamp::Now();
+    do {
+      rv = mRemoteLock.Lock(mRemoteLockDir, nullptr);
+      if (NS_SUCCEEDED(rv)) break;
+      PR_Sleep(START_SLEEP_MSEC);
+    } while ((mozilla::TimeStamp::Now() - epoch) <
+             mozilla::TimeDuration::FromSeconds(START_TIMEOUT_SEC));
+    if (NS_FAILED(rv)) {
+      NS_WARNING("Cannot lock remote start mutex");
+    }
+  }
+}
+
+void nsRemoteService::UnlockStartup() {
+  mRemoteLock.Unlock();
+  mRemoteLock.Cleanup();
+
+  if (mRemoteLockDir) {
+    mRemoteLockDir->Remove(false);
+    mRemoteLockDir = nullptr;
+  }
+}
+
 RemoteResult nsRemoteService::StartClient(const char* aDesktopStartupID,
                                           nsCString& program,
                                           const char* profile) {
   nsAutoPtr<nsRemoteClient> client;
 
 #ifdef MOZ_WIDGET_GTK
   bool useX11Remote = GDK_IS_X11_DISPLAY(gdk_display_get_default());
 
--- a/toolkit/components/remote/nsRemoteService.h
+++ b/toolkit/components/remote/nsRemoteService.h
@@ -8,35 +8,42 @@
 #ifndef __nsRemoteService_h__
 #define __nsRemoteService_h__
 
 #include "nsRemoteServer.h"
 #include "nsIObserverService.h"
 #include "nsIObserver.h"
 #include "nsPIDOMWindow.h"
 #include "mozilla/UniquePtr.h"
+#include "nsIFile.h"
+#include "nsProfileLock.h"
 
 enum RemoteResult {
   REMOTE_NOT_FOUND = 0,
   REMOTE_FOUND = 1,
   REMOTE_ARG_BAD = 2
 };
 
 class nsRemoteService final : public nsIObserver {
  public:
   // We will be a static singleton, so don't use the ordinary methods.
   NS_DECL_ISUPPORTS
   NS_DECL_NSIOBSERVER
 
   nsRemoteService() = default;
 
+  void LockStartup(nsCString& aProgram, const char* aProfile);
+  void UnlockStartup();
+
   RemoteResult StartClient(const char* aDesktopStartupID, nsCString& program,
                            const char* profile);
   void StartupServer(const char* aAppName, const char* aProfileName);
   void ShutdownServer();
 
  private:
   ~nsRemoteService();
 
   mozilla::UniquePtr<nsRemoteServer> mRemoteServer;
+  nsProfileLock mRemoteLock;
+  nsCOMPtr<nsIFile> mRemoteLockDir;
 };
 
 #endif  // __nsRemoteService_h__
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -191,18 +191,16 @@
 #  include "nsXRemoteClient.h"
 #  include "nsRemoteService.h"
 #  include "nsProfileLock.h"
 #  include "SpecialSystemDirectory.h"
 #  include <sched.h>
 #  ifdef MOZ_ENABLE_DBUS
 #    include "nsDBusRemoteClient.h"
 #  endif
-// Time to wait for the remoting service to start
-#  define MOZ_XREMOTE_START_TIMEOUT_SEC 5
 #endif
 
 #if defined(DEBUG) && defined(XP_WIN32)
 #  include <malloc.h>
 #endif
 
 #if defined(XP_MACOSX)
 #  include <Carbon/Carbon.h>
@@ -2877,18 +2875,16 @@ class XREMain {
 
   nsCOMPtr<nsINativeAppSupport> mNativeApp;
   RefPtr<nsToolkitProfileService> mProfileSvc;
   nsCOMPtr<nsIFile> mProfD;
   nsCOMPtr<nsIFile> mProfLD;
   nsCOMPtr<nsIProfileLock> mProfileLock;
 #if defined(MOZ_WIDGET_GTK)
   RefPtr<nsRemoteService> mRemoteService;
-  nsProfileLock mRemoteLock;
-  nsCOMPtr<nsIFile> mRemoteLockDir;
 #endif
 
   UniquePtr<ScopedXPCOMStartup> mScopedXPCOM;
   UniquePtr<XREAppData> mAppData;
 
   nsXREDirProvider mDirProvider;
   nsAutoCString mProfileName;
   nsAutoCString mDesktopStartupID;
@@ -3914,45 +3910,17 @@ int XREMain::XRE_mainStartup(bool* aExit
       nsAutoCString program(gAppData->remotingName);
       ToLowerCase(program);
 
       const char* profile = nullptr;
       // It doesn't matter if this succeeds or fails. A failure will be handled
       // later.
       CheckArg("p", &profile, CheckArgFlag::None);
 
-      nsCOMPtr<nsIFile> mutexDir;
-      rv = GetSpecialSystemDirectory(OS_TemporaryDirectory,
-                                     getter_AddRefs(mutexDir));
-      if (NS_SUCCEEDED(rv)) {
-        nsAutoCString mutexPath = program;
-        if (profile) {
-          mutexPath.Append(NS_LITERAL_CSTRING("_") +
-                           nsDependentCString(profile));
-        }
-        mutexDir->AppendNative(mutexPath);
-
-        rv = mutexDir->Create(nsIFile::DIRECTORY_TYPE, 0700);
-        if (NS_SUCCEEDED(rv) || rv == NS_ERROR_FILE_ALREADY_EXISTS) {
-          mRemoteLockDir = mutexDir;
-        }
-      }
-
-      if (mRemoteLockDir) {
-        const TimeStamp epoch = mozilla::TimeStamp::Now();
-        do {
-          rv = mRemoteLock.Lock(mRemoteLockDir, nullptr);
-          if (NS_SUCCEEDED(rv)) break;
-          sched_yield();
-        } while ((TimeStamp::Now() - epoch) <
-                 TimeDuration::FromSeconds(MOZ_XREMOTE_START_TIMEOUT_SEC));
-        if (NS_FAILED(rv)) {
-          NS_WARNING("Cannot lock XRemote start mutex");
-        }
-      }
+      mRemoteService->LockStartup(program, profile);
 
       // Try to remote the entire command line. If this fails, start up
       // normally.
       const char* desktopStartupIDPtr =
           mDesktopStartupID.IsEmpty() ? nullptr : mDesktopStartupID.get();
 
       RemoteResult rr =
           mRemoteService->StartClient(desktopStartupIDPtr, program, profile);
@@ -4587,21 +4555,17 @@ nsresult XREMain::XRE_mainRun() {
   }
 
   if (!mShuttingDown) {
 #if defined(MOZ_WIDGET_GTK)
     // if we have X remote support, start listening for requests on the
     // proxy window.
     if (mRemoteService) {
       mRemoteService->StartupServer(mAppData->remotingName, mProfileName.get());
-    }
-    if (mRemoteLockDir) {
-      mRemoteLock.Unlock();
-      mRemoteLock.Cleanup();
-      mRemoteLockDir->Remove(false);
+      mRemoteService->UnlockStartup();
     }
 #endif /* MOZ_WIDGET_GTK */
 
     mNativeApp->Enable();
   }
 
 #ifdef MOZ_INSTRUMENT_EVENT_LOOP
   if (PR_GetEnv("MOZ_INSTRUMENT_EVENT_LOOP")) {