adb: fix subprocess exit handling, oom adjust fixes, extra debugging.
* Add support for correctly handling subprocess termination in shell service (b/3400254 b/3482112 b/2249397)
- have a waitpid() track the subprocess, then notify the fdevent via a socket
- force an eof on the pty master in fdevent's new subproc handler.
- modify fdevent to force-read the pty after an exit.
* Migrate the "shell:blabla" handling to "#if !ADB_HOST" sections, where it
belongs.
* Fix the race around OOM adjusting.
- Do it in the child before exec() instead of the in the parent as the
child could already have started or not (no /proc/pid/... yet).
* Allow for multi-threaded D() invocations to not clobber each other.
- Allow locks across object files.
- Add lock within D()
- Make sure sysdesp init (mutex init also) is called early.
* Add some missing close(fd) calls
- Match similar existing practices near dup2()
* Add extra D() invocations related to FD handling.
* Warn about using debugging as stderr/stdout is used for protocol.
* Fix some errno handling and make D() correctly handle it.
* Add new adb trace_mask: services.
* Make fdevent_loop's handle BADFDs more gracefully (could occur some subproc closed its pts explicitely).
* Remove obsolete commandline args reported in help. (b/3509092)
Change-Id: I928287fdf4f1a86777e22ce105f9581685f46e35
diff --git a/adb/services.c b/adb/services.c
index 7eab17a..43f9174 100644
--- a/adb/services.c
+++ b/adb/services.c
@@ -22,7 +22,7 @@
#include "sysdeps.h"
-#define TRACE_TAG TRACE_ADB
+#define TRACE_TAG TRACE_SERVICES
#include "adb.h"
#include "file_sync_service.h"
@@ -30,6 +30,7 @@
# ifndef HAVE_WINSOCK
# include <netinet/in.h>
# include <netdb.h>
+# include <sys/ioctl.h>
# endif
#else
# include <cutils/android_reboot.h>
@@ -267,15 +268,16 @@
return s[0];
}
-static int create_subprocess(const char *cmd, const char *arg0, const char *arg1)
+#if !ADB_HOST
+static int create_subprocess(const char *cmd, const char *arg0, const char *arg1, pid_t *pid)
{
#ifdef HAVE_WIN32_PROC
- fprintf(stderr, "error: create_subprocess not implemented on Win32 (%s %s %s)\n", cmd, arg0, arg1);
- return -1;
+ D("create_subprocess(cmd=%s, arg0=%s, arg1=%s)\n", cmd, arg0, arg1);
+ fprintf(stderr, "error: create_subprocess not implemented on Win32 (%s %s %s)\n", cmd, arg0, arg1);
+ return -1;
#else /* !HAVE_WIN32_PROC */
char *devname;
int ptm;
- pid_t pid;
ptm = unix_open("/dev/ptmx", O_RDWR); // | O_NOCTTY);
if(ptm < 0){
@@ -287,22 +289,27 @@
if(grantpt(ptm) || unlockpt(ptm) ||
((devname = (char*) ptsname(ptm)) == 0)){
printf("[ trouble with /dev/ptmx - %s ]\n", strerror(errno));
+ adb_close(ptm);
return -1;
}
- pid = fork();
- if(pid < 0) {
+ *pid = fork();
+ if(*pid < 0) {
printf("- fork failed: %s -\n", strerror(errno));
+ adb_close(ptm);
return -1;
}
- if(pid == 0){
+ if(*pid == 0){
int pts;
setsid();
pts = unix_open(devname, O_RDWR);
- if(pts < 0) exit(-1);
+ if(pts < 0) {
+ fprintf(stderr, "child failed to open pseudo-term slave: %s\n", devname);
+ exit(-1);
+ }
dup2(pts, 0);
dup2(pts, 1);
@@ -311,15 +318,9 @@
adb_close(pts);
adb_close(ptm);
- execl(cmd, cmd, arg0, arg1, NULL);
- fprintf(stderr, "- exec '%s' failed: %s (%d) -\n",
- cmd, strerror(errno), errno);
- exit(-1);
- } else {
-#if !ADB_HOST
- // set child's OOM adjustment to zero
+ // set OOM adjustment to zero
char text[64];
- snprintf(text, sizeof text, "/proc/%d/oom_adj", pid);
+ snprintf(text, sizeof text, "/proc/%d/oom_adj", getpid());
int fd = adb_open(text, O_WRONLY);
if (fd >= 0) {
adb_write(fd, "0", 1);
@@ -327,11 +328,20 @@
} else {
D("adb: unable to open %s\n", text);
}
-#endif
+ execl(cmd, cmd, arg0, arg1, NULL);
+ fprintf(stderr, "- exec '%s' failed: %s (%d) -\n",
+ cmd, strerror(errno), errno);
+ exit(-1);
+ } else {
+ // Don't set child's OOM adjustment to zero.
+ // Let the child do it itself, as sometimes the parent starts
+ // running before the child has a /proc/pid/oom_adj.
+ // """adb: unable to open /proc/644/oom_adj""" seen in some logs.
return ptm;
}
#endif /* !HAVE_WIN32_PROC */
}
+#endif /* !ABD_HOST */
#if ADB_HOST
#define SHELL_COMMAND "/bin/sh"
@@ -339,6 +349,70 @@
#define SHELL_COMMAND "/system/bin/sh"
#endif
+#if !ADB_HOST
+static void subproc_waiter_service(int fd, void *cookie)
+{
+ pid_t pid = (pid_t)cookie;
+
+ D("entered. fd=%d of pid=%d\n", fd, pid);
+ for (;;) {
+ int status;
+ pid_t p = waitpid(pid, &status, 0);
+ if (p == pid) {
+ D("fd=%d, post waitpid(pid=%d) status=%04x\n", fd, p, status);
+ if (WIFSIGNALED(status)) {
+ D("*** Killed by signal %d\n", WTERMSIG(status));
+ break;
+ } else if (!WIFEXITED(status)) {
+ D("*** Didn't exit!!. status %d\n", status);
+ break;
+ } else if (WEXITSTATUS(status) >= 0) {
+ D("*** Exit code %d\n", WEXITSTATUS(status));
+ break;
+ }
+ }
+ usleep(100000); // poll every 0.1 sec
+ }
+ D("shell exited fd=%d of pid=%d err=%d\n", fd, pid, errno);
+ if (SHELL_EXIT_NOTIFY_FD >=0) {
+ int res;
+ res = writex(SHELL_EXIT_NOTIFY_FD, &fd, sizeof(fd));
+ D("notified shell exit via fd=%d for pid=%d res=%d errno=%d\n",
+ SHELL_EXIT_NOTIFY_FD, pid, res, errno);
+ }
+}
+
+static int create_subproc_thread(const char *name)
+{
+ stinfo *sti;
+ adb_thread_t t;
+ int ret_fd;
+ pid_t pid;
+ if(name) {
+ ret_fd = create_subprocess(SHELL_COMMAND, "-c", name, &pid);
+ } else {
+ ret_fd = create_subprocess(SHELL_COMMAND, "-", 0, &pid);
+ }
+ D("create_subprocess() ret_fd=%d pid=%d\n", ret_fd, pid);
+
+ sti = malloc(sizeof(stinfo));
+ if(sti == 0) fatal("cannot allocate stinfo");
+ sti->func = subproc_waiter_service;
+ sti->cookie = (void*)pid;
+ sti->fd = ret_fd;
+
+ if(adb_thread_create( &t, service_bootstrap_func, sti)){
+ free(sti);
+ adb_close(ret_fd);
+ printf("cannot create service thread\n");
+ return -1;
+ }
+
+ D("service thread started, fd=%d pid=%d\n",ret_fd, pid);
+ return ret_fd;
+}
+#endif
+
int service_to_fd(const char *name)
{
int ret = -1;
@@ -389,14 +463,12 @@
ret = create_jdwp_connection_fd(atoi(name+5));
} else if (!strncmp(name, "log:", 4)) {
ret = create_service_thread(log_service, get_log_file_path(name + 4));
-#endif
} else if(!HOST && !strncmp(name, "shell:", 6)) {
if(name[6]) {
- ret = create_subprocess(SHELL_COMMAND, "-c", name + 6);
+ ret = create_subproc_thread(name + 6);
} else {
- ret = create_subprocess(SHELL_COMMAND, "-", 0);
+ ret = create_subproc_thread(0);
}
-#if !ADB_HOST
} else if(!strncmp(name, "sync:", 5)) {
ret = create_service_thread(file_sync_service, NULL);
} else if(!strncmp(name, "remount:", 8)) {