Chromoting: Stop the Mac daemon if a permanent error is encountered

Launchd is no longer responsible for reloading the daemon. This responsibility
now belongs to the helper script which will compare the host's exit code against
the permanent error range to decide whether the host should be restarted.

BUG=123715
TEST=manual: enable, disable, sigterm host, sigterm script, change pin, force segfault host, force auth-error host, disable & reboot, enable & reboot 


Review URL: http://codereview.chromium.org/10383003

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

4 files changed, 88 insertions, 32 deletions

diff --git a/remoting/host/constants.h b/remoting/host/constants.h
index 6f36b94..9654aff 100644
--- a/remoting/host/constants.h
+++ b/remoting/host/constants.h
@@ -8,6 +8,9 @@
 namespace remoting {
 
 // Known host exit codes.
+// Please keep this enum in sync with:
+// remoting/host/installer/mac/PrivilegedHelperTools/
+// org.chromium.chromoting.me2me.sh
 enum HostExitCodes {
   kSuccessExitCode = 0,
   kReservedForX11ExitCode = 1,
diff --git a/remoting/host/installer/mac/LaunchAgents/org.chromium.chromoting.plist b/remoting/host/installer/mac/LaunchAgents/org.chromium.chromoting.plist
index 5e74439..a7ca612 100644
--- a/remoting/host/installer/mac/LaunchAgents/org.chromium.chromoting.plist
+++ b/remoting/host/installer/mac/LaunchAgents/org.chromium.chromoting.plist
@@ -9,14 +9,8 @@
 		<string>Aqua</string>
 		<string>LoginWindow</string>
 	</array>
-	<key>KeepAlive</key>
-	<dict>
-		<key>PathState</key>
-		<dict>
-			<key>/Library/PrivilegedHelperTools/org.chromium.chromoting.me2me_enabled</key>
-			<true/>
-		</dict>
-	</dict>
+	<key>RunAtLoad</key>
+	<true/>
 	<key>Disabled</key>
 	<false/>
 	<key>ProgramArguments</key>
diff --git a/remoting/host/installer/mac/PrivilegedHelperTools/org.chromium.chromoting.me2me.sh b/remoting/host/installer/mac/PrivilegedHelperTools/org.chromium.chromoting.me2me.sh
index a033362..28369e2 100755
--- a/remoting/host/installer/mac/PrivilegedHelperTools/org.chromium.chromoting.me2me.sh
+++ b/remoting/host/installer/mac/PrivilegedHelperTools/org.chromium.chromoting.me2me.sh
@@ -10,21 +10,78 @@ HOST_EXE=$CONFIG_DIR/$NAME.me2me_host
 ENABLED_FILE=$CONFIG_DIR/$NAME.me2me_enabled
 CONFIG_FILE=$CONFIG_DIR/$NAME.json
 
-if [ "$1" = "--disable" ]; then
+# The exit code returned by 'wait' when a process is terminated by SIGTERM.
+SIGTERM_EXIT_CODE=143
+
+# Range of exit codes returned by the host to indicate that a permanent error
+# has occurred and that the host should not be restarted. Please, keep these
+# constants in sync with remoting/host/constants.h.
+MIN_PERMANENT_ERROR_EXIT_CODE=2
+MAX_PERMANENT_ERROR_EXIT_CODE=4
+
+HOST_PID=0
+SIGNAL_WAS_TRAPPED=0
+
+handle_signal() {
+  SIGNAL_WAS_TRAPPED=1
+}
+
+run_host() {
+  # This script works as a proxy between launchd and the host. Signals of
+  # interest to the host must be forwarded.
+  trap "handle_signal" SIGHUP SIGINT SIGQUIT SIGILL SIGTRAP SIGABRT SIGEMT \
+      SIGFPE SIGKILL SIGBUS SIGSEGV SIGSYS SIGPIPE SIGALRM SIGTERM SIGURG \
+      SIGSTOP SIGTSTP SIGCONT SIGCHLD SIGTTIN SIGTTOU SIGIO SIGXCPU SIGXFSZ \
+      SIGVTALRM SIGPROF SIGWINCH SIGINFO SIGUSR1 SIGUSR2
+
+  while true; do
+    if [[ ! -f "$ENABLED_FILE" ]]; then
+      echo "Daemon is disabled."
+      exit 0
+    fi
+
+    # Execute the host asynchronously
+    "$HOST_EXE" --auth-config="$CONFIG_FILE" --host-config="$CONFIG_FILE" &
+    HOST_PID="$!"
+
+    # Wait for the host to return and process its exit code.
+    while true; do
+      wait "$HOST_PID"
+      EXIT_CODE="$?"
+      if [[ $SIGNAL_WAS_TRAPPED -eq 1 ]]; then
+        # 'wait' returned as the result of a trapped signal and the exit code is
+        # the signal that was trapped + 128. Forward the signal to the host.
+        SIGNAL_WAS_TRAPPED=0
+        local SIGNAL=$(($EXIT_CODE - 128))
+        echo "Forwarding signal $SIGNAL to host"
+        kill -$SIGNAL "$HOST_PID"
+      elif [[ "$EXIT_CODE" -eq "$SIGTERM_EXIT_CODE" ||
+              ("$EXIT_CODE" -ge "$MIN_PERMANENT_ERROR_EXIT_CODE" && \
+              "$EXIT_CODE" -le "$MAX_PERMANENT_ERROR_EXIT_CODE") ]]; then
+        echo "Host returned permanent exit code $EXIT_CODE"
+        exit "$EXIT_CODE"
+      else
+        # Ignore non-permanent error-code and launch host again.
+        echo "Host returned non-permanent exit code $EXIT_CODE"
+        break
+      fi
+    done
+  done
+}
+
+if [[ "$1" = "--disable" ]]; then
   # This script is executed from base::mac::ExecuteWithPrivilegesAndWait(),
   # which requires the child process to write its PID to stdout before
   # anythine else. See base/mac/authorization_util.h for details.
   echo $$
   rm -f "$ENABLED_FILE"
-elif [ "$1" = "--enable" ]; then
+elif [[ "$1" = "--enable" ]]; then
   echo $$
   cat > "$CONFIG_FILE"
   touch "$ENABLED_FILE"
-elif [ "$1" = "--save-config" ]; then
+elif [[ "$1" = "--save-config" ]]; then
   echo $$
   cat > "$CONFIG_FILE"
 else
-  exec "$HOST_EXE" \
-    --auth-config="$CONFIG_FILE" \
-    --host-config="$CONFIG_FILE"
+  run_host
 fi
diff --git a/remoting/host/plugin/daemon_controller_mac.cc b/remoting/host/plugin/daemon_controller_mac.cc
index fb62be7..a75d613 100644
--- a/remoting/host/plugin/daemon_controller_mac.cc
+++ b/remoting/host/plugin/daemon_controller_mac.cc
@@ -34,12 +34,8 @@ namespace {
 #define kConfigDir "/Library/PrivilegedHelperTools/"
 
 // This helper script is executed as root.  It is passed a command-line option
-// (--enable or --disable), which causes it to create or remove a trigger file.
-// The trigger file (defined in the service's plist file) informs launchd
-// whether the Host service should be running.  Creating the trigger file causes
-// launchd to immediately start the service.  Deleting the trigger file has no
-// immediate effect, but it prevents the service from being restarted if it
-// becomes stopped.
+// (--enable or --disable), which causes it to create or remove a file that
+// informs the host's launch script of whether the host is enabled or disabled.
 const char kStartStopTool[] = kConfigDir kServiceName ".me2me.sh";
 
 // Use a single configuration file, instead of separate "auth" and "host" files.
@@ -78,7 +74,7 @@ class DaemonControllerMac : public remoting::DaemonController {
                          const base::TimeDelta& sleep);
 
   bool RunToolScriptAsRoot(const char* command, const std::string& input_data);
-  bool StopService();
+  bool SendJobControlMessage(const char* launchKey);
 
   // The API for gaining root privileges is blocking (it prompts the user for
   // a password). Since Start() and Stop() must not block the main thread, they
@@ -173,10 +169,11 @@ void DaemonControllerMac::DoSetConfigAndStart(
     const CompletionCallback& done_callback) {
   std::string file_content;
   base::JSONWriter::Write(config.get(), &file_content);
-
-  // Creating the trigger file causes launchd to start the service, so the
-  // extra step performed in DoStop() is not necessary here.
-  bool result = RunToolScriptAsRoot("--enable", file_content);
+  if (!RunToolScriptAsRoot("--enable", file_content)) {
+    done_callback.Run(RESULT_FAILED);
+    return;
+  }
+  bool result = SendJobControlMessage(LAUNCH_KEY_STARTJOB);
   done_callback.Run(result ? RESULT_OK : RESULT_FAILED);
 }
 
@@ -187,6 +184,7 @@ void DaemonControllerMac::DoUpdateConfig(
   JsonHostConfig config_file(config_file_path);
   if (!config_file.Read()) {
     done_callback.Run(RESULT_FAILED);
+    return;
   }
   for (DictionaryValue::key_iterator key(config->begin_keys());
        key != config->end_keys(); ++key) {
@@ -194,14 +192,18 @@ void DaemonControllerMac::DoUpdateConfig(
     if (!config->GetString(*key, &value)) {
       LOG(ERROR) << *key << " is not a string.";
       done_callback.Run(RESULT_FAILED);
+      return;
     }
     config_file.SetString(*key, value);
   }
 
   std::string file_content = config_file.GetSerializedData();
-  bool success = RunToolScriptAsRoot("--save-config", file_content);
+  if (!RunToolScriptAsRoot("--save-config", file_content)) {
+      done_callback.Run(RESULT_FAILED);
+      return;
+  }
 
-  done_callback.Run(success ? RESULT_OK : RESULT_FAILED);
+  done_callback.Run(RESULT_OK);
   pid_t job_pid = base::mac::PIDForJob(kServiceName);
   if (job_pid > 0) {
     kill(job_pid, SIGHUP);
@@ -218,13 +220,13 @@ void DaemonControllerMac::DoStop(const CompletionCallback& done_callback) {
   // Since the service is running for the local user's desktop (not as root),
   // it has to be stopped for that user.  This cannot easily be done in the
   // shell-script running as root, so it is done here instead.
-  if (!StopService()) {
+  if (!SendJobControlMessage(LAUNCH_KEY_STOPJOB)) {
     done_callback.Run(RESULT_FAILED);
     return;
   }
 
-  // StopService does not wait for the stop to take effect, so we can't return
-  // immediately. Instead, we wait up to 10s.
+  // SendJobControlMessage does not wait for the stop to take effect, so we
+  // can't return immediately. Instead, we wait up to 10s.
   NotifyWhenStopped(done_callback,
                     kStopWaitRetryLimit,
                     base::TimeDelta::FromMilliseconds(kStopWaitTimeout));
@@ -315,9 +317,9 @@ bool DaemonControllerMac::RunToolScriptAsRoot(const char* command,
   }
 }
 
-bool DaemonControllerMac::StopService() {
+bool DaemonControllerMac::SendJobControlMessage(const char* launchKey) {
   base::mac::ScopedLaunchData response(
-      base::mac::MessageForJob(kServiceName, LAUNCH_KEY_STOPJOB));
+      base::mac::MessageForJob(kServiceName, launchKey));
   if (!response) {
     LOG(ERROR) << "Failed to send message to launchd";
     return false;