adb: refactor subprocess code.

Refactor shell_service.cpp to remove dependencies on service.cpp and
combine some common logic between PTY and raw subprocesses.

This will make it easier to add additional common code paths for
the upcoming shell protocol.

Change-Id: I497d30dd388de61b6e68d9086dce38f33dd92876
diff --git a/shell_service.cpp b/shell_service.cpp
index 6fdbbdb..5f80a59 100644
--- a/shell_service.cpp
+++ b/shell_service.cpp
@@ -20,9 +20,12 @@
 
 #if !ADB_HOST
 
+#include <errno.h>
 #include <pty.h>
 #include <termios.h>
 
+#include <base/logging.h>
+#include <base/stringprintf.h>
 #include <paths.h>
 
 #include "adb.h"
@@ -46,182 +49,281 @@
     }
 }
 
-int create_subproc_pty(const char* cmd, const char* arg0, const char* arg1,
-                       pid_t* pid) {
-    D("create_subproc_pty(cmd=%s, arg0=%s, arg1=%s)", cmd, arg0, arg1);
-    char pts_name[PATH_MAX];
-    int ptm;
-    *pid = forkpty(&ptm, pts_name, nullptr, nullptr);
-    if (*pid == -1) {
-        printf("- fork failed: %s -\n", strerror(errno));
-        unix_close(ptm);
-        return -1;
+// Reads from |fd| until close or failure.
+std::string ReadAll(int fd) {
+    char buffer[512];
+    std::string received;
+
+    while (1) {
+        int bytes = adb_read(fd, buffer, sizeof(buffer));
+        if (bytes <= 0) {
+            break;
+        }
+        received.append(buffer, bytes);
     }
 
-    if (*pid == 0) {
+    return received;
+}
+
+// Helper to automatically close an FD when it goes out of scope.
+class ScopedFd {
+  public:
+    ScopedFd() {}
+    ~ScopedFd() { Reset(); }
+
+    void Reset(int fd=-1) {
+        if (fd != fd_) {
+            if (valid()) {
+                adb_close(fd_);
+            }
+            fd_ = fd;
+        }
+    }
+
+    int Release() {
+        int temp = fd_;
+        fd_ = -1;
+        return temp;
+    }
+
+    bool valid() const { return fd_ >= 0; }
+
+    int fd() const { return fd_; }
+
+  private:
+    int fd_ = -1;
+
+    DISALLOW_COPY_AND_ASSIGN(ScopedFd);
+};
+
+// Creates a socketpair and saves the endpoints to |fd1| and |fd2|.
+bool CreateSocketpair(ScopedFd* fd1, ScopedFd* fd2) {
+    int sockets[2];
+    if (adb_socketpair(sockets) < 0) {
+        PLOG(ERROR) << "cannot create socket pair";
+        return false;
+    }
+    fd1->Reset(sockets[0]);
+    fd2->Reset(sockets[1]);
+    return true;
+}
+
+class Subprocess {
+  public:
+    Subprocess(const std::string& command, SubprocessType type);
+    ~Subprocess();
+
+    const std::string& command() const { return command_; }
+    bool is_interactive() const { return command_.empty(); }
+
+    int local_socket_fd() const { return local_socket_sfd_.fd(); }
+
+    pid_t pid() const { return pid_; }
+
+    // Sets up FDs, forks a subprocess, starts the subprocess manager thread,
+    // and exec's the child. Returns false on failure.
+    bool ForkAndExec();
+
+  private:
+    // Opens the file at |pts_name|.
+    int OpenPtyChildFd(const char* pts_name, ScopedFd* error_sfd);
+
+    static void* ThreadHandler(void* userdata);
+    void WaitForExit();
+
+    const std::string command_;
+    SubprocessType type_;
+
+    pid_t pid_ = -1;
+    ScopedFd local_socket_sfd_;
+
+    DISALLOW_COPY_AND_ASSIGN(Subprocess);
+};
+
+Subprocess::Subprocess(const std::string& command, SubprocessType type)
+        : command_(command), type_(type) {
+}
+
+Subprocess::~Subprocess() {
+}
+
+bool Subprocess::ForkAndExec() {
+    ScopedFd parent_sfd, child_sfd, parent_error_sfd, child_error_sfd;
+    char pts_name[PATH_MAX];
+
+    // Create a socketpair for the fork() child to report any errors back to
+    // the parent. Since we use threads, logging directly from the child could
+    // create a race condition.
+    if (!CreateSocketpair(&parent_error_sfd, &child_error_sfd)) {
+        LOG(ERROR) << "failed to create pipe for subprocess error reporting";
+    }
+
+    if (type_ == SubprocessType::kPty) {
+        int fd;
+        pid_ = forkpty(&fd, pts_name, nullptr, nullptr);
+        parent_sfd.Reset(fd);
+    } else {
+        if (!CreateSocketpair(&parent_sfd, &child_sfd)) {
+            return false;
+        }
+        pid_ = fork();
+    }
+
+    if (pid_ == -1) {
+        PLOG(ERROR) << "fork failed";
+        return false;
+    }
+
+    if (pid_ == 0) {
+        // Subprocess child.
         init_subproc_child();
 
-        int pts = unix_open(pts_name, O_RDWR | O_CLOEXEC);
-        if (pts == -1) {
-            fprintf(stderr, "child failed to open pseudo-term slave %s: %s\n",
-                    pts_name, strerror(errno));
-            unix_close(ptm);
+        if (type_ == SubprocessType::kPty) {
+            child_sfd.Reset(OpenPtyChildFd(pts_name, &child_error_sfd));
+        }
+
+        dup2(child_sfd.fd(), STDIN_FILENO);
+        dup2(child_sfd.fd(), STDOUT_FILENO);
+        dup2(child_sfd.fd(), STDERR_FILENO);
+
+        // exec doesn't trigger destructors, close the FDs manually.
+        parent_sfd.Reset();
+        child_sfd.Reset();
+        parent_error_sfd.Reset();
+        close_on_exec(child_error_sfd.fd());
+
+        if (is_interactive()) {
+            execl(_PATH_BSHELL, _PATH_BSHELL, "-", nullptr);
+        } else {
+            execl(_PATH_BSHELL, _PATH_BSHELL, "-c", command_.c_str(), nullptr);
+        }
+        WriteFdExactly(child_error_sfd.fd(), "exec '" _PATH_BSHELL "' failed");
+        child_error_sfd.Reset();
+        exit(-1);
+    }
+
+    // Subprocess parent.
+    D("subprocess parent: subprocess FD = %d", parent_sfd.fd());
+
+    // Wait to make sure the subprocess exec'd without error.
+    child_error_sfd.Reset();
+    std::string error_message = ReadAll(parent_error_sfd.fd());
+    if (!error_message.empty()) {
+        LOG(ERROR) << error_message;
+        return false;
+    }
+
+    local_socket_sfd_.Reset(parent_sfd.Release());
+
+    if (!adb_thread_create(ThreadHandler, this)) {
+        PLOG(ERROR) << "failed to create subprocess thread";
+        return false;
+    }
+
+    return true;
+}
+
+int Subprocess::OpenPtyChildFd(const char* pts_name, ScopedFd* error_sfd) {
+    int child_fd = adb_open(pts_name, O_RDWR | O_CLOEXEC);
+    if (child_fd == -1) {
+        // Don't use WriteFdFmt; since we're in the fork() child we don't want
+        // to allocate any heap memory to avoid race conditions.
+        const char* messages[] = {"child failed to open pseudo-term slave ",
+                                  pts_name, ": ", strerror(errno)};
+        for (const char* message : messages) {
+            WriteFdExactly(error_sfd->fd(), message);
+        }
+        exit(-1);
+    }
+
+    if (!is_interactive()) {
+        termios tattr;
+        if (tcgetattr(child_fd, &tattr) == -1) {
+            WriteFdExactly(error_sfd->fd(), "tcgetattr failed");
             exit(-1);
         }
 
-        // arg0 is "-c" in batch mode and "-" in interactive mode.
-        if (strcmp(arg0, "-c") == 0) {
-            termios tattr;
-            if (tcgetattr(pts, &tattr) == -1) {
-                fprintf(stderr, "tcgetattr failed: %s\n", strerror(errno));
-                unix_close(pts);
-                unix_close(ptm);
-                exit(-1);
-            }
-
-            cfmakeraw(&tattr);
-            if (tcsetattr(pts, TCSADRAIN, &tattr) == -1) {
-                fprintf(stderr, "tcsetattr failed: %s\n", strerror(errno));
-                unix_close(pts);
-                unix_close(ptm);
-                exit(-1);
-            }
+        cfmakeraw(&tattr);
+        if (tcsetattr(child_fd, TCSADRAIN, &tattr) == -1) {
+            WriteFdExactly(error_sfd->fd(), "tcsetattr failed");
+            exit(-1);
         }
-
-        dup2(pts, STDIN_FILENO);
-        dup2(pts, STDOUT_FILENO);
-        dup2(pts, STDERR_FILENO);
-
-        unix_close(pts);
-        unix_close(ptm);
-
-        execl(cmd, cmd, arg0, arg1, nullptr);
-        fprintf(stderr, "- exec '%s' failed: %s (%d) -\n",
-                cmd, strerror(errno), errno);
-        exit(-1);
-    } else {
-        return ptm;
     }
+
+    return child_fd;
 }
 
-int create_subproc_raw(const char *cmd, const char *arg0, const char *arg1,
-                       pid_t *pid)
-{
-    D("create_subproc_raw(cmd=%s, arg0=%s, arg1=%s)", cmd, arg0, arg1);
+void* Subprocess::ThreadHandler(void* userdata) {
+    Subprocess* subprocess = reinterpret_cast<Subprocess*>(userdata);
 
-    // 0 is parent socket, 1 is child socket
-    int sv[2];
-    if (adb_socketpair(sv) < 0) {
-        printf("[ cannot create socket pair - %s ]\n", strerror(errno));
-        return -1;
-    }
-    D("socketpair: (%d,%d)", sv[0], sv[1]);
+    adb_thread_setname(android::base::StringPrintf(
+            "shell srvc %d", subprocess->local_socket_fd()));
 
-    *pid = fork();
-    if (*pid < 0) {
-        printf("- fork failed: %s -\n", strerror(errno));
-        adb_close(sv[0]);
-        adb_close(sv[1]);
-        return -1;
-    }
+    subprocess->WaitForExit();
 
-    if (*pid == 0) {
-        adb_close(sv[0]);
-        init_subproc_child();
+    D("deleting Subprocess");
+    delete subprocess;
 
-        dup2(sv[1], STDIN_FILENO);
-        dup2(sv[1], STDOUT_FILENO);
-        dup2(sv[1], STDERR_FILENO);
-
-        adb_close(sv[1]);
-
-        execl(cmd, cmd, arg0, arg1, NULL);
-        fprintf(stderr, "- exec '%s' failed: %s (%d) -\n",
-                cmd, strerror(errno), errno);
-        exit(-1);
-    } else {
-        adb_close(sv[1]);
-        return sv[0];
-    }
+    return nullptr;
 }
 
-void subproc_waiter_service(int fd, void *cookie)
-{
-    pid_t pid = (pid_t) (uintptr_t) cookie;
-
-    D("entered. fd=%d of pid=%d", fd, pid);
+void Subprocess::WaitForExit() {
+    D("waiting for pid %d", pid_);
     while (true) {
         int status;
-        pid_t p = waitpid(pid, &status, 0);
-        if (p == pid) {
-            D("fd=%d, post waitpid(pid=%d) status=%04x", fd, p, status);
+        if (pid_ == waitpid(pid_, &status, 0)) {
+            D("post waitpid (pid=%d) status=%04x", pid_, status);
             if (WIFSIGNALED(status)) {
-                D("*** Killed by signal %d", WTERMSIG(status));
+                D("subprocess killed by signal %d", WTERMSIG(status));
                 break;
             } else if (!WIFEXITED(status)) {
-                D("*** Didn't exit!!. status %d", status);
+                D("subprocess didn't exit");
                 break;
             } else if (WEXITSTATUS(status) >= 0) {
-                D("*** Exit code %d", WEXITSTATUS(status));
+                D("subprocess exit code = %d", WEXITSTATUS(status));
                 break;
             }
-         }
+        }
     }
-    D("shell exited fd=%d of pid=%d err=%d", fd, pid, errno);
-    if (SHELL_EXIT_NOTIFY_FD >=0) {
-      int res;
-      res = WriteFdExactly(SHELL_EXIT_NOTIFY_FD, &fd, sizeof(fd)) ? 0 : -1;
-      D("notified shell exit via fd=%d for pid=%d res=%d errno=%d",
-        SHELL_EXIT_NOTIFY_FD, pid, res, errno);
+
+    // Pass the local socket FD to the shell cleanup fdevent.
+    if (SHELL_EXIT_NOTIFY_FD >= 0) {
+        int fd = local_socket_sfd_.fd();
+        if (WriteFdExactly(SHELL_EXIT_NOTIFY_FD, &fd, sizeof(fd))) {
+            D("passed fd %d to SHELL_EXIT_NOTIFY_FD (%d) for pid %d",
+              fd, SHELL_EXIT_NOTIFY_FD, pid_);
+            // The shell exit fdevent now owns the FD and will close it once
+            // the last bit of data flushes through.
+            local_socket_sfd_.Release();
+        } else {
+            PLOG(ERROR) << "failed to write fd " << fd
+                        << " to SHELL_EXIT_NOTIFY_FD (" << SHELL_EXIT_NOTIFY_FD
+                        << ") for pid " << pid_;
+        }
     }
 }
 
 }  // namespace
 
-// References to things in services.cpp. This is temporary just to make as few
-// modifications as possible while moving functionality into it's own file.
-// TODO(dpursell): remove these in the next CL.
-void *service_bootstrap_func(void *x);
-struct stinfo_duplicate {
-    void (*func)(int fd, void *cookie);
-    int fd;
-    void *cookie;
-};
-
 int StartSubprocess(const char *name, SubprocessType type) {
-    const char *arg0, *arg1;
-    if (*name == '\0') {
-        arg0 = "-";
-        arg1 = nullptr;
-    } else {
-        arg0 = "-c";
-        arg1 = name;
-    }
+    D("starting %s subprocess: '%s'",
+      type == SubprocessType::kRaw ? "raw" : "PTY", name);
 
-    pid_t pid = -1;
-    int ret_fd;
-    if (type == SubprocessType::kPty) {
-        ret_fd = create_subproc_pty(_PATH_BSHELL, arg0, arg1, &pid);
-    } else {
-        ret_fd = create_subproc_raw(_PATH_BSHELL, arg0, arg1, &pid);
-    }
-    D("create_subproc ret_fd=%d pid=%d", ret_fd, pid);
-
-    stinfo_duplicate* sti = reinterpret_cast<stinfo_duplicate*>(malloc(sizeof(stinfo_duplicate)));
-    if(sti == 0) fatal("cannot allocate stinfo");
-    sti->func = subproc_waiter_service;
-    sti->cookie = (void*) (uintptr_t) pid;
-    sti->fd = ret_fd;
-
-    if (!adb_thread_create(service_bootstrap_func, sti)) {
-        free(sti);
-        adb_close(ret_fd);
-        fprintf(stderr, "cannot create service thread\n");
+    Subprocess* subprocess = new Subprocess(name, type);
+    if (!subprocess) {
+        LOG(ERROR) << "failed to allocate new subprocess";
         return -1;
     }
 
-    D("service thread started, fd=%d pid=%d", ret_fd, pid);
-    return ret_fd;
+    if (!subprocess->ForkAndExec()) {
+        LOG(ERROR) << "failed to start subprocess";
+        delete subprocess;
+        return -1;
+    }
+
+    D("subprocess creation successful: local_socket_fd=%d, pid=%d",
+      subprocess->local_socket_fd(), subprocess->pid());
+    return subprocess->local_socket_fd();
 }
 
 #endif  // !ADB_HOST