Give each process its own bootstrap subset port as a subset of its inherited

bootstrap port, and use the bootstrap subset port as the bootstrap port.

This completely eliminates leaks of on-demand Mach services advertised via the
bootstrap server.

This also reverts r34318 and r34534, "temporary" (21-month) hacks to mitigate
the leak. The temporary hacks were never completely effective against Breakpad
ports leaking from child processes.

DestructCrashReporter at process shutdown was deemed unnecessary and is being
removed. As this was the last caller to that function, the implementation is
removed as well.

This is addressed in Chrome rather than Breakpad to account for the potential
leak of rohitfork ports if the browser process crashes or is mercilessly
killed, because library code messing with the task's bootstrap port doesn't
strike me as kosher, and because the Mac Breakpad code is scheduled to be
replaced with something better that doesn't attempt to leak ports like a sieve
within a couple of months anyway.

BUG=28547
TEST=1. "launchctl bslist" should no longer show on-demand
        com.Breakpad.Inspector ports (in Breakpad-enabled builds with Breakpad
        on) or com.google.Chrome.rohitfork, com.google.Chrome.canary.rohitfork,
        or org.chromium.Chromium.rohitfork ports.
     2. "launchctl bstree" (as root) should reveal a bootstrap subset for the
        browser process as a child of the per-user/per-session bootstrap
        namespace containing the rohitfork port and browser's Breakpad port if
        crash reporting is on. There should also be a bootstrap subset for 
        each child process as a child of the browser's bootstrap subset. If
        crash reporting is on, each child process' bootstrap subset should 
        contain a Breakpad port.
     3. Breakpad reports should be generated on crashes. For example,
        about:crash and about:inducebrowsercrashforrealz should each cause a
        minidump to be written in
        ~/Library/Application Support/Google/Chrome/Crash Reports
        when Breakpad is enabled. This tests that the Breakpad ports are
        functioning properly.
     4. The browser process should be able to access child process data.
        Window:Task Manager should show valid values for the Memory, CPU, and
        Network columns for all child processes. This tests that the rohitfork
        port is functioning properly.
Review URL: http://codereview.chromium.org/8059041

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@103089 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 69 insertions, 158 deletions

diff --git a/chrome/app/breakpad_mac.h b/chrome/app/breakpad_mac.h
index b737f0d..8ff1d26 100644
--- a/chrome/app/breakpad_mac.h
+++ b/chrome/app/breakpad_mac.h
@@ -19,7 +19,4 @@ void InitCrashProcessInfo();
 // Is Breakpad enabled?
 bool IsCrashReporterEnabled();
 
-// Call on clean process shutdown.
-void DestructCrashReporter();
-
 #endif  // CHROME_APP_BREAKPAD_MAC_H_
diff --git a/chrome/app/breakpad_mac.mm b/chrome/app/breakpad_mac.mm
index 4f4d11c..f078271 100644
--- a/chrome/app/breakpad_mac.mm
+++ b/chrome/app/breakpad_mac.mm
@@ -56,13 +56,6 @@ bool IsCrashReporterEnabled() {
   return gBreakpadRef != NULL;
 }
 
-void DestructCrashReporter() {
-  if (gBreakpadRef) {
-    BreakpadRelease(gBreakpadRef);
-    gBreakpadRef = NULL;
-  }
-}
-
 // Only called for a branded build of Chrome.app.
 void InitCrashReporter() {
   DCHECK(!gBreakpadRef);
diff --git a/chrome/app/breakpad_mac_stubs.mm b/chrome/app/breakpad_mac_stubs.mm
index db2db6b..f8a651a 100644
--- a/chrome/app/breakpad_mac_stubs.mm
+++ b/chrome/app/breakpad_mac_stubs.mm
@@ -16,8 +16,5 @@ bool IsCrashReporterEnabled() {
 void InitCrashProcessInfo() {
 }
 
-void DestructCrashReporter() {
-}
-
 void InitCrashReporter() {
 }
diff --git a/chrome/app/chrome_main.cc b/chrome/app/chrome_main.cc
index 96f4b51..777b723 100644
--- a/chrome/app/chrome_main.cc
+++ b/chrome/app/chrome_main.cc
@@ -387,6 +387,8 @@ class ChromeMainDelegate : public content::ContentMainDelegate {
     DCHECK(ObjcEvilDoers::ZombieEnable(true, 1000));
     SetUpBundleOverrides();
     chrome::common::mac::EnableCFBundleBlocker();
+
+    SwitchToMachBootstrapSubsetPort();
 #endif
 
     Profiling::ProcessStarted();
@@ -679,11 +681,6 @@ class ChromeMainDelegate : public content::ContentMainDelegate {
       ResourceBundle::CleanupSharedInstance();
 
     logging::CleanupChromeLogging();
-
-#if defined(OS_MACOSX) && defined(GOOGLE_CHROME_BUILD)
-    // TODO(mark): See the TODO(mark) at InitCrashReporter.
-    DestructCrashReporter();
-#endif  // OS_MACOSX && GOOGLE_CHROME_BUILD
   }
 
 #if defined(OS_MACOSX)
diff --git a/chrome/app/chrome_main_mac.h b/chrome/app/chrome_main_mac.h
index 6fafd47..2ceeb03 100644
--- a/chrome/app/chrome_main_mac.h
+++ b/chrome/app/chrome_main_mac.h
@@ -19,4 +19,27 @@ void CheckUserDataDirPolicy(FilePath* user_data_dir);
 // process.
 void SetUpBundleOverrides();
 
+// Creates a bootstrap subset port as a subset of the current bootstrap port,
+// tying its lifetime to the current task port, and switches the task's
+// bootstrap port to the new bootstrap subset port. Any subsequent bootstrap
+// servers created in the task via bootstrap_create_server and directed at the
+// bootstrap port will be created in the bootstrap subset port, meaning that
+// they will only be visible via bootstrap_look_up to this task and its
+// children, and that these mappings will be destroyed along with the subset
+// port as soon as this process exits.
+//
+// This scheme prevents bootstrap server mappings from leaking beyond this
+// process' lifetime. It also prevents any mappings from being visible by any
+// process other than this one and its children, but currently, nothing
+// requires this behavior. If anything ever does, this function could save the
+// original bootstrap port and make it available to things that need to call
+// bootstrap_create_server and create mappings with the original bootstrap
+// port.
+//
+// This needs to be called before anything calls bootstrap_create_server.
+// Currently, the only things that create bootstrap server mappings are
+// Breakpad and rohitfork.  To look for other users, search for
+// bootstrap_create_server and -[NSMachBootstrapServer registerPort:name:].
+void SwitchToMachBootstrapSubsetPort();
+
 #endif  // CHROME_APP_CHROME_MAIN_MAC_H_
diff --git a/chrome/app/chrome_main_mac.mm b/chrome/app/chrome_main_mac.mm
index a09f9d6..d6607e0 100644
--- a/chrome/app/chrome_main_mac.mm
+++ b/chrome/app/chrome_main_mac.mm
@@ -5,6 +5,8 @@
 #include "chrome/app/chrome_main_mac.h"
 
 #import <Cocoa/Cocoa.h>
+#include <mach/mach.h>
+#include <servers/bootstrap.h>
 
 #include <string>
 
@@ -43,3 +45,45 @@ void SetUpBundleOverrides() {
   NSBundle* base_bundle = chrome::OuterAppBundle();
   base::mac::SetBaseBundleID([[base_bundle bundleIdentifier] UTF8String]);
 }
+
+void SwitchToMachBootstrapSubsetPort() {
+  // Testing tip: use launchctl bstree (as root) to make sure that the
+  // subset port is created properly and that new mappings wind up added to
+  // the subset port.
+
+#ifndef NDEBUG
+  static bool once_only = false;
+  DCHECK(!once_only);
+  once_only = true;
+#endif
+
+  mach_port_t self_task = mach_task_self();
+
+  mach_port_t original_bootstrap_port;
+  kern_return_t kr = task_get_bootstrap_port(self_task,
+                                             &original_bootstrap_port);
+  if (kr != KERN_SUCCESS) {
+    LOG(ERROR) << "task_get_bootstrap_port: " << kr << " "
+               << mach_error_string(kr);
+    return;
+  }
+
+  mach_port_t bootstrap_subset_port;
+  kr = bootstrap_subset(original_bootstrap_port,
+                        self_task,
+                        &bootstrap_subset_port);
+  if (kr != BOOTSTRAP_SUCCESS) {
+    LOG(ERROR) << "bootstrap_subset: " << kr << " " << bootstrap_strerror(kr);
+    return;
+  }
+
+  kr = task_set_bootstrap_port(self_task, bootstrap_subset_port);
+  if (kr != KERN_SUCCESS) {
+    LOG(ERROR) << "task_set_bootstrap_port: " << kr << " "
+               << mach_error_string(kr);
+    return;
+  }
+
+  // Users of the bootstrap port often access it through this global variable.
+  bootstrap_port = bootstrap_subset_port;
+}
diff --git a/chrome/renderer/chrome_render_process_observer.cc b/chrome/renderer/chrome_render_process_observer.cc
index a1ec2b3..93d1fa8 100644
--- a/chrome/renderer/chrome_render_process_observer.cc
+++ b/chrome/renderer/chrome_render_process_observer.cc
@@ -47,11 +47,6 @@
 #include "base/win/iat_patch_function.h"
 #endif
 
-#if defined(OS_MACOSX)
-#include "base/eintr_wrapper.h"
-#include "chrome/app/breakpad_mac.h"
-#endif
-
 using WebKit::WebCache;
 using WebKit::WebCrossOriginPreflightResultCache;
 using WebKit::WebFontCache;
@@ -190,121 +185,11 @@ class SuicideOnChannelErrorFilter : public IPC::ChannelProxy::MessageFilter {
     // So, we install a filter on the channel so that we can process this event
     // here and kill the process.
 
-#if defined(OS_MACOSX)
-    // TODO(viettrungluu): crbug.com/28547: The following is needed, as a
-    // stopgap, to avoid leaking due to not releasing Breakpad properly.
-    // TODO(viettrungluu): Investigate why this is being called.
-    if (IsCrashReporterEnabled()) {
-      VLOG(1) << "Cleaning up Breakpad.";
-      DestructCrashReporter();
-    } else {
-      VLOG(1) << "Breakpad not enabled; no clean-up needed.";
-    }
-#endif  // OS_MACOSX
-
     _exit(0);
   }
 };
 #endif  // OS_POSIX
 
-#if defined(OS_MACOSX)
-// TODO(viettrungluu): crbug.com/28547: The following signal handling is needed,
-// as a stopgap, to avoid leaking due to not releasing Breakpad properly.
-// Without this problem, this could all be eliminated. Remove when Breakpad is
-// fixed?
-// TODO(viettrungluu): Code taken from browser_main.cc (with a bit of editing).
-// The code should be properly shared (or this code should be eliminated).
-int g_shutdown_pipe_write_fd = -1;
-
-void SIGTERMHandler(int signal) {
-  RAW_CHECK(signal == SIGTERM);
-
-  // Reinstall the default handler.  We had one shot at graceful shutdown.
-  struct sigaction action;
-  memset(&action, 0, sizeof(action));
-  action.sa_handler = SIG_DFL;
-  CHECK(sigaction(signal, &action, NULL) == 0);
-
-  RAW_CHECK(g_shutdown_pipe_write_fd != -1);
-  size_t bytes_written = 0;
-  do {
-    int rv = HANDLE_EINTR(
-        write(g_shutdown_pipe_write_fd,
-              reinterpret_cast<const char*>(&signal) + bytes_written,
-              sizeof(signal) - bytes_written));
-    RAW_CHECK(rv >= 0);
-    bytes_written += rv;
-  } while (bytes_written < sizeof(signal));
-}
-
-class ShutdownDetector : public base::PlatformThread::Delegate {
- public:
-  explicit ShutdownDetector(int shutdown_fd) : shutdown_fd_(shutdown_fd) {
-    CHECK(shutdown_fd_ != -1);
-  }
-
-  virtual void ThreadMain() {
-    int signal;
-    size_t bytes_read = 0;
-    ssize_t ret;
-    do {
-      ret = HANDLE_EINTR(
-          read(shutdown_fd_,
-               reinterpret_cast<char*>(&signal) + bytes_read,
-               sizeof(signal) - bytes_read));
-      if (ret < 0) {
-        NOTREACHED() << "Unexpected error: " << strerror(errno);
-        break;
-      } else if (ret == 0) {
-        NOTREACHED() << "Unexpected closure of shutdown pipe.";
-        break;
-      }
-      bytes_read += ret;
-    } while (bytes_read < sizeof(signal));
-
-    if (bytes_read == sizeof(signal))
-      VLOG(1) << "Handling shutdown for signal " << signal << ".";
-    else
-      VLOG(1) << "Handling shutdown for unknown signal.";
-
-    // Clean up Breakpad if necessary.
-    if (IsCrashReporterEnabled()) {
-      VLOG(1) << "Cleaning up Breakpad.";
-      DestructCrashReporter();
-    } else {
-      VLOG(1) << "Breakpad not enabled; no clean-up needed.";
-    }
-
-    // Something went seriously wrong, so get out.
-    if (bytes_read != sizeof(signal)) {
-      LOG(WARNING) << "Failed to get signal. Quitting ungracefully.";
-      _exit(1);
-    }
-
-    // Re-raise the signal.
-    kill(getpid(), signal);
-
-    // The signal may be handled on another thread. Give that a chance to
-    // happen.
-    sleep(3);
-
-    // We really should be dead by now.  For whatever reason, we're not. Exit
-    // immediately, with the exit status set to the signal number with bit 8
-    // set.  On the systems that we care about, this exit status is what is
-    // normally used to indicate an exit by this signal's default handler.
-    // This mechanism isn't a de jure standard, but even in the worst case, it
-    // should at least result in an immediate exit.
-    LOG(WARNING) << "Still here, exiting really ungracefully.";
-    _exit(signal | (1 << 7));
-  }
-
- private:
-  const int shutdown_fd_;
-
-  DISALLOW_COPY_AND_ASSIGN(ShutdownDetector);
-};
-#endif  // OS_MACOSX
-
 }  // namespace
 
 bool ChromeRenderProcessObserver::is_incognito_process_ = false;
@@ -329,31 +214,6 @@ ChromeRenderProcessObserver::ChromeRenderProcessObserver(
   thread->AddFilter(new SuicideOnChannelErrorFilter());
 #endif
 
-#if defined(OS_MACOSX)
-  // TODO(viettrungluu): Code taken from browser_main.cc.
-  int pipefd[2];
-  int ret = pipe(pipefd);
-  if (ret < 0) {
-    PLOG(DFATAL) << "Failed to create pipe";
-  } else {
-    int shutdown_pipe_read_fd = pipefd[0];
-    g_shutdown_pipe_write_fd = pipefd[1];
-    const size_t kShutdownDetectorThreadStackSize = 4096;
-    if (!base::PlatformThread::CreateNonJoinable(
-            kShutdownDetectorThreadStackSize,
-            new ShutdownDetector(shutdown_pipe_read_fd))) {
-      LOG(DFATAL) << "Failed to create shutdown detector task.";
-    }
-  }
-
-  // crbug.com/28547: When Breakpad is in use, handle SIGTERM to avoid leaking
-  // Mach ports.
-  struct sigaction action;
-  memset(&action, 0, sizeof(action));
-  action.sa_handler = SIGTERMHandler;
-  CHECK(sigaction(SIGTERM, &action, NULL) == 0);
-#endif
-
   // Configure modules that need access to resources.
   net::NetModule::SetResourceProvider(chrome_common_net::NetResourceProvider);