Support HTTP test-server based net unit tests on Android.

tools/android/forwarder2/daemon.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "tools/android/forwarder2/daemon.h"
+
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdio.h>
+#include <sys/file.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/eintr_wrapper.h"
+#include "base/file_path.h"
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "base/safe_strerror_posix.h"
+#include "base/string_number_conversions.h"
+#include "base/stringprintf.h"
+#include "tools/android/forwarder2/common.h"
+
+namespace forwarder2 {
+namespace {
+
+const char kLogFilePath[] = "/tmp/host_forwarder_log";
+
+class FileDescriptorAutoCloser {
+ public:
+  explicit FileDescriptorAutoCloser(int fd) : fd_(fd) {
+    DCHECK(fd_ >= 0);
+  }
+
+  ~FileDescriptorAutoCloser() {
+    if (fd_ > -1)
+      CloseFD(fd_);
+  }
+
+  int Release() {
+    const int fd = fd_;
+    fd_ = -1;
+    return fd;
+  }
+
+ private:
+  int fd_;
+
+  DISALLOW_COPY_AND_ASSIGN(FileDescriptorAutoCloser);
+};
+
+// Handles creation and destruction of the PID file.
+class PIDFile {
+ public:
+  static scoped_ptr<PIDFile> Create(const std::string& path) {
+    scoped_ptr<PIDFile> pid_file;
+    const int pid_file_fd = HANDLE_EINTR(
+        open(path.c_str(), O_CREAT | O_WRONLY, 0600));

[digit1, 2012/11/06 10:39:24]

Note: there is a slight race condition here between the creation of the PID file
and the read in Kill() below.

It's probably to create a PID file first with a completely random name, then to
do an unlink() + rename() to ensure that it will appear automatically to the
reader process.

[Philippe, 2012/11/06 16:13:31]

I did what we talked about offline. I'm now fetching the daemon's PID through
the file lock to avoid the race condition.

+    if (pid_file_fd < 0) {
+      PError("open()");
+      return pid_file.Pass();
+    }
+    FileDescriptorAutoCloser fd_closer(pid_file_fd);
+    if (HANDLE_EINTR(flock(pid_file_fd, LOCK_EX | LOCK_NB)) < 0) {
+      if (errno == EAGAIN || errno == EACCES) {
+        LOG(ERROR) << "Daemon already running (PID file already locked)";
+        return pid_file.Pass();
+      }
+      PError("lockf()");
+      return pid_file.Pass();
+    }
+    const std::string pid_string = base::StringPrintf("%d\n", getpid());
+    CHECK(HANDLE_EINTR(write(pid_file_fd, pid_string.c_str(),
+                             pid_string.length())));
+    pid_file.reset(new PIDFile(fd_closer.Release(), path));
+    return pid_file.Pass();
+  }
+
+  ~PIDFile() {
+    CloseFD(fd_);  // This also releases the lock.
+    if (remove(path_.c_str()) < 0)
+      PError("remove");
+  }
+
+ private:
+  PIDFile(int fd, const std::string& path) : fd_(fd), path_(path) {
+    DCHECK(fd_ >= 0);
+  }
+
+  const int fd_;
+  const std::string path_;
+
+  DISALLOW_COPY_AND_ASSIGN(PIDFile);
+};
+
+// Takes ownership of |data|.
+void ReleaseDaemonResourcesAtExit(void* data) {
+  DCHECK(data);
+  delete reinterpret_cast<PIDFile*>(data);
+}
+
+void InitLogging(const char* log_file) {
+  CHECK(
+      logging::InitLogging(
+          log_file,
+          logging::LOG_ONLY_TO_FILE,
+          logging::DONT_LOCK_LOG_FILE,
+          logging::APPEND_TO_OLD_LOG_FILE,
+          logging::ENABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS));
+}
+
+void SigChildHandler(int signal_number) {
+  DCHECK_EQ(signal_number, SIGCHLD);
+  // The daemon should not terminate while its parent is still running.
+  int status;
+  pid_t child_pid = waitpid(-1 /* any child */, &status, WNOHANG);
+  if (child_pid < 0) {
+    PError("waitpid");
+    return;
+  }
+  if (child_pid == 0)
+    return;
+  // Avoid using StringAppendF() since it's unsafe in a signal handler due to
+  // its use of LOG().
+  FixedSizeStringBuilder<256> string_builder;
+  string_builder.Append("Daemon (pid=%d) died unexpectedly with ", child_pid);
+  if (WIFEXITED(status))
+    string_builder.Append("status %d.", WEXITSTATUS(status));
+  else if (WIFSIGNALED(status))
+    string_builder.Append("signal %d.", WTERMSIG(status));
+  else
+    string_builder.Append("unknown reason.");
+  SIGNAL_SAFE_LOG(ERROR, string_builder.buffer());
+}
+
+}  // namespace
+
+Daemon::Daemon(const std::string& pid_file_path)
+    : pid_file_path_(pid_file_path) {
+}
+
+bool Daemon::Spawn(bool* is_daemon) {
+  switch (fork()) {

[digit1, 2012/11/06 10:39:24]

Do you need a double-fork here to avoid zombies?

See http://fixunix.com/unix/533215-how-avoid-zombie-processes.html

[Philippe, 2012/11/06 16:13:31]

As we discussed offline I think the current implementation should prevent
zombies.
My understanding is that processes go to the zombie state when their parent
doesn't wait for them when they die.
Here we correctly catch the SIG_CHLD and do the wait if the daemon dies before
its parent (the CLI process). If the parent dies before the child (which is the
expected behavior since the daemon is supposed to live longer than the CLI
process) then the daemon is reattached to the init process which will do the
wait.

+    case -1:
+      *is_daemon = false;
+      PError("fork()");
+      return false;
+    case 0: {  // Child.
+      *is_daemon = true;
+      scoped_ptr<PIDFile> pid_file = PIDFile::Create(pid_file_path_);
+      if (!pid_file)
+        return false;
+      base::AtExitManager::RegisterCallback(
+          &ReleaseDaemonResourcesAtExit, pid_file.release());
+      if (setsid() < 0) {  // Detach the child process from its parent.
+        PError("setsid");
+        return false;
+      }
+      CloseFD(STDOUT_FILENO);
+      CloseFD(STDERR_FILENO);
+      InitLogging(kLogFilePath);
+      break;
+    }
+    default:  // Parent.
+      *is_daemon = false;
+      signal(SIGCHLD, SigChildHandler);
+  }
+  return true;
+}
+
+bool Daemon::Kill() {
+  std::string pid_string;
+  const FilePath pid_file_path(pid_file_path_);
+  if (!file_util::ReadFileToString(pid_file_path, &pid_string)) {
+    int error = errno;
+    if (file_util::PathExists(pid_file_path)) {
+      LOG(ERROR) << "Could not read file " << pid_file_path_ << ": "
+                 << safe_strerror(error);
+      return false;
+    }
+    // Reasonably assume that the daemon is not running.
+    return true;
+  }
+  CHECK(pid_string.length() > 1);
+  // Remove the trailing \n.
+  pid_string.resize(pid_string.length() - 1);
+  pid_t pid;
+  CHECK(base::StringToInt(pid_string, &pid));
+  CHECK_NE(pid, getpid());
+  if (kill(pid, SIGTERM) < 0) {
+    DCHECK_EQ(ESRCH /* invalid PID */, errno);
+    // The daemon is not running and didn't remove its PID file for some reason.
+    // We don't treat this as an error.
+    if (remove(pid_file_path_.c_str()) < 0)
+      PError("remove");
+    return true;
+  }
+  int pid_file_fd = HANDLE_EINTR(open(pid_file_path_.c_str(), O_WRONLY));
+  if (pid_file_fd < 0) {
+    if (errno == ENOENT)
+      // Note that the file was present when ReadFileToString() was called above
+      // so assume that it was just removed by the daemon between the call to
+      // kill() above and here.
+      return true;
+    LOG(ERROR) << "Could not open " << pid_file_path_ << " in write mode: "
+               << safe_strerror(errno);
+    return false;
+  }
+  const FileDescriptorAutoCloser fd_closer(pid_file_fd);
+  // Wait until the daemon exits. Rely on the fact that the daemon releases the
+  // lock on the PID file when it exits.
+  // TODO(pliard): Consider using a mutex + condition in shared memory to avoid
+  // polling.
+  const int kTries = 20;
+  const int kIdleTimeMS = 100;

[Philippe, 2012/11/06 16:13:31]

I decreased this value which was a bit high.

+  for (int i = 0; i < kTries; ++i) {
+    struct flock lock_info = {};
+    lock_info.l_type = F_WRLCK;
+    lock_info.l_whence = SEEK_CUR;
+    const int ret = HANDLE_EINTR(fcntl(pid_file_fd, F_GETLK, &lock_info));
+    if (ret < 0)
+      PError("fcntl");
+    else if (lock_info.l_type == F_UNLCK)
+      return true;
+    else {
+      CHECK_EQ(F_WRLCK /* exclusive lock */, lock_info.l_type);
+      if (lock_info.l_pid != pid) {
+        LOG(WARNING) << "Daemon (pid=" << pid
+                     << ") was successfully killed but a new daemon (pid="
+                     << lock_info.l_pid << ") seems to be running now.";
+        return true;
+      }
+    }
+    usleep(kIdleTimeMS * 1000);
+  }
+  LOG(ERROR) << "Timed out while killing daemon. "
+                "It might still be tearing down.";
+  return false;
+}
+
+}  // namespace forwarder2