Backed out changeset 30de9ac21a78 (bug 1256992) for causing crashes a=backout
authorWes Kocher <wkocher@mozilla.com>
Mon, 28 Mar 2016 10:51:05 -0700
changeset 323653 b0b8d69d265bd7700c684fce54eccf7cddde403f
parent 323652 5629a22932f74f3e2d647c3462c32191a791afed
child 323654 261a56c9e3487b42c3e73418f69ec51609afa505
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1256992
milestone47.0a2
backs out30de9ac21a783228a1bb6b3d5920050824cbc58f
Backed out changeset 30de9ac21a78 (bug 1256992) for causing crashes a=backout MozReview-Commit-ID: L30XEzrSo0y
js/xpconnect/src/XPCShellImpl.cpp
security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
security/sandbox/win/src/sandboxbroker/sandboxBroker.h
toolkit/xre/nsAppRunner.cpp
--- a/js/xpconnect/src/XPCShellImpl.cpp
+++ b/js/xpconnect/src/XPCShellImpl.cpp
@@ -39,19 +39,16 @@
 
 #ifdef ANDROID
 #include <android/log.h>
 #endif
 
 #ifdef XP_WIN
 #include "mozilla/widget/AudioSession.h"
 #include <windows.h>
-#if defined(MOZ_SANDBOX)
-#include "SandboxBroker.h"
-#endif
 #endif
 
 // all this crap is needed to do the interactive shell stuff
 #include <stdlib.h>
 #include <errno.h>
 #ifdef HAVE_IO_H
 #include <io.h>     /* for isatty() */
 #endif
@@ -1516,24 +1513,16 @@ XRE_XPCShellMain(int argc, char** argv, 
         // Initialize graphics prefs on the main thread, if not already done
         gfxPrefs::GetSingleton();
         // Initialize e10s check on the main thread, if not already done
         BrowserTabsRemoteAutostart();
 #ifdef XP_WIN
         // Plugin may require audio session if installed plugin can initialize
         // asynchronized.
         AutoAudioSession audioSession;
-
-#if defined(MOZ_SANDBOX)
-        // Required for sandboxed child processes.
-        if (!SandboxBroker::Initialize()) {
-          NS_WARNING("Failed to initialize broker services, sandboxed "
-                     "processes will fail to start.");
-        }
-#endif
 #endif
 
         {
             JS::Rooted<JSObject*> glob(cx, holder->GetJSObject());
             if (!glob) {
                 return 1;
             }
 
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
@@ -12,40 +12,31 @@
 #include "sandbox/win/src/security_level.h"
 #include "mozilla/sandboxing/sandboxLogging.h"
 
 namespace mozilla
 {
 
 sandbox::BrokerServices *SandboxBroker::sBrokerService = nullptr;
 
-/* static */
-bool
-SandboxBroker::Initialize()
+SandboxBroker::SandboxBroker()
 {
-  sBrokerService = sandbox::SandboxFactory::GetBrokerServices();
+  // XXX: This is not thread-safe! Two threads could simultaneously try
+  // to set `sBrokerService`
   if (!sBrokerService) {
-    return false;
+    sBrokerService = sandbox::SandboxFactory::GetBrokerServices();
+    if (sBrokerService) {
+      sandbox::ResultCode result = sBrokerService->Init();
+      if (result != sandbox::SBOX_ALL_OK) {
+        sBrokerService = nullptr;
+      }
+    }
   }
 
-  if (sBrokerService->Init() != sandbox::SBOX_ALL_OK) {
-    sBrokerService = nullptr;
-    return false;
-  }
-
-  return true;
-}
-
-SandboxBroker::SandboxBroker()
-{
-  if (sBrokerService) {
-    mPolicy = sBrokerService->CreatePolicy();
-  } else {
-    mPolicy = nullptr;
-  }
+  mPolicy = sBrokerService->CreatePolicy();
 }
 
 bool
 SandboxBroker::LaunchApp(const wchar_t *aPath,
                          const wchar_t *aArguments,
                          const bool aEnableLogging,
                          void **aProcessHandle)
 {
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h
@@ -22,19 +22,16 @@ namespace sandbox {
 }
 
 namespace mozilla {
 
 class SANDBOX_EXPORT SandboxBroker
 {
 public:
   SandboxBroker();
-
-  static bool Initialize();
-
   bool LaunchApp(const wchar_t *aPath,
                  const wchar_t *aArguments,
                  const bool aEnableLogging,
                  void **aProcessHandle);
   virtual ~SandboxBroker();
 
   // Security levels for different types of processes
 #if defined(MOZ_CONTENT_SANDBOX)
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -204,22 +204,18 @@
 #ifdef MOZ_B2G_LOADER
 #include "ProcessUtils.h"
 #endif
 
 #ifdef MOZ_WIDGET_ANDROID
 #include "AndroidBridge.h"
 #endif
 
-#if defined(MOZ_SANDBOX)
-#if defined(XP_LINUX) && !defined(ANDROID)
+#if defined(MOZ_SANDBOX) && defined(XP_LINUX) && !defined(ANDROID)
 #include "mozilla/SandboxInfo.h"
-#elif defined(XP_WIN)
-#include "SandboxBroker.h"
-#endif
 #endif
 
 extern uint32_t gRestartMode;
 extern void InstallSignalHandlers(const char *ProgramName);
 
 #define FILE_COMPATIBILITY_INFO NS_LITERAL_CSTRING("compatibility.ini")
 #define FILE_INVALIDATE_CACHES NS_LITERAL_CSTRING(".purgecaches")
 
@@ -3750,22 +3746,16 @@ XREMain::XRE_mainStartup(bool* aExitFlag
   if (!gtk_parse_args(&gArgc, &gArgv))
     return 1;
 #endif /* MOZ_WIDGET_GTK */
 
   if (PR_GetEnv("MOZ_RUN_GTEST")) {
     int result;
 #ifdef XP_WIN
     UseParentConsole();
-#if defined(MOZ_SANDBOX)
-    if (!SandboxBroker::Initialize()) {
-      NS_WARNING("Failed to initialize broker services, sandboxed processes "
-                 "will fail to start.");
-    }
-#endif
 #endif
     // RunGTest will only be set if we're in xul-unit
     if (mozilla::RunGTest) {
       gIsGtest = true;
       result = mozilla::RunGTest();
       gIsGtest = false;
     } else {
       result = 1;
@@ -4341,30 +4331,16 @@ XREMain::XRE_mainRun()
 
 #ifdef MOZ_INSTRUMENT_EVENT_LOOP
   if (PR_GetEnv("MOZ_INSTRUMENT_EVENT_LOOP")) {
     bool logToConsole = true;
     mozilla::InitEventTracing(logToConsole);
   }
 #endif /* MOZ_INSTRUMENT_EVENT_LOOP */
 
-#if defined(MOZ_SANDBOX) && defined(XP_WIN)
-  if (!SandboxBroker::Initialize()) {
-#if defined(MOZ_CONTENT_SANDBOX)
-    // If we're sandboxing content and we fail to initialize, then crashing here
-    // seems like the sensible option.
-    if (BrowserTabsRemoteAutostart()) {
-      MOZ_CRASH("Failed to initialize broker services, can't continue.");
-    }
-#endif
-    // Otherwise just warn for the moment, as most things will work.
-    NS_WARNING("Failed to initialize broker services, sandboxed processes will "
-               "fail to start.");
-  }
-#endif
 #if (defined(XP_WIN) || defined(XP_MACOSX)) && defined(MOZ_CONTENT_SANDBOX)
   SetUpSandboxEnvironment();
 #endif
 
   {
     rv = appStartup->Run();
     if (NS_FAILED(rv)) {
       NS_ERROR("failed to run appstartup");