Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 408fa578 authored by JP Abgrall's avatar JP Abgrall
Browse files

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
parent cdae7a1d
Loading
Loading
Loading
Loading
+15 −11
Original line number Diff line number Diff line
@@ -36,6 +36,9 @@
#include "usb_vendors.h"
#endif

#if ADB_TRACE
ADB_MUTEX_DEFINE( D_lock );
#endif

int HOST = 0;

@@ -90,6 +93,7 @@ void adb_trace_init(void)
        { "sysdeps", TRACE_SYSDEPS },
        { "transport", TRACE_TRANSPORT },
        { "jdwp", TRACE_JDWP },
        { "services", TRACE_SERVICES },
        { NULL, 0 }
    };

@@ -591,14 +595,6 @@ nomem:
    return 0;
}

#ifdef HAVE_FORKEXEC
static void sigchld_handler(int n)
{
    int status;
    while(waitpid(-1, &status, WNOHANG) > 0) ;
}
#endif

#ifdef HAVE_WIN32_PROC
static BOOL WINAPI ctrlc_handler(DWORD type)
{
@@ -641,6 +637,7 @@ void start_logging(void)

    fd = unix_open("/dev/null", O_RDONLY);
    dup2(fd, 0);
    adb_close(fd);

    fd = unix_open("/tmp/adb.log", O_WRONLY | O_CREAT | O_APPEND, 0640);
    if(fd < 0) {
@@ -648,6 +645,7 @@ void start_logging(void)
    }
    dup2(fd, 1);
    dup2(fd, 2);
    adb_close(fd);
    fprintf(stderr,"--- adb starting (pid %d) ---\n", getpid());
#endif
}
@@ -807,9 +805,10 @@ int launch_server(int server_port)
        // wait for the "OK\n" message
        adb_close(fd[1]);
        int ret = adb_read(fd[0], temp, 3);
        int saved_errno = errno;
        adb_close(fd[0]);
        if (ret < 0) {
            fprintf(stderr, "could not read ok from ADB Server, errno = %d\n", errno);
            fprintf(stderr, "could not read ok from ADB Server, errno = %d\n", saved_errno);
            return -1;
        }
        if (ret != 3 || temp[0] != 'O' || temp[1] != 'K' || temp[2] != '\n') {
@@ -848,7 +847,7 @@ int adb_main(int is_daemon, int server_port)
#ifdef HAVE_WIN32_PROC
    SetConsoleCtrlHandler( ctrlc_handler, TRUE );
#elif defined(HAVE_FORKEXEC)
    signal(SIGCHLD, sigchld_handler);
    // No SIGCHLD. Let the service subproc handle its children.
    signal(SIGPIPE, SIG_IGN);
#endif

@@ -957,7 +956,9 @@ int adb_main(int is_daemon, int server_port)
        // listen on default port
        local_init(DEFAULT_ADB_LOCAL_TRANSPORT_PORT);
    }
    D("adb_main(): pre init_jdwp()\n");
    init_jdwp();
    D("adb_main(): post init_jdwp()\n");
#endif

    if (is_daemon)
@@ -971,6 +972,7 @@ int adb_main(int is_daemon, int server_port)
#endif
        start_logging();
    }
    D("Event loop starting\n");

    fdevent_loop();

@@ -1269,8 +1271,9 @@ int recovery_mode = 0;
int main(int argc, char **argv)
{
#if ADB_HOST
    adb_trace_init();
    adb_sysdeps_init();
    adb_trace_init();
    D("Handling commandline()\n");
    return adb_commandline(argc - 1, argv + 1);
#else
    if((argc > 1) && (!strcmp(argv[1],"recovery"))) {
@@ -1279,6 +1282,7 @@ int main(int argc, char **argv)
    }

    start_device_log();
    D("Handling main()\n");
    return adb_main(0, DEFAULT_ADB_PORT);
#endif
}
+33 −14
Original line number Diff line number Diff line
@@ -19,6 +19,8 @@

#include <limits.h>

#include "transport.h"  /* readx(), writex() */

#define MAX_PAYLOAD 4096

#define A_SYNC 0x434e5953
@@ -315,13 +317,6 @@ void put_apacket(apacket *p);
int check_header(apacket *p);
int check_data(apacket *p);

/* convenience wrappers around read/write that will retry on
** EINTR and/or short read/write.  Returns 0 on success, -1
** on error or EOF.
*/
int readx(int fd, void *ptr, size_t len);
int writex(int fd, const void *ptr, size_t len);

/* define ADB_TRACE to 1 to enable tracing support, or 0 to disable it */

#define  ADB_TRACE    1
@@ -331,21 +326,22 @@ int writex(int fd, const void *ptr, size_t len);
 * the adb_trace_init() function implemented in adb.c
 */
typedef enum {
    TRACE_ADB = 0,
    TRACE_ADB = 0,   /* 0x001 */
    TRACE_SOCKETS,
    TRACE_PACKETS,
    TRACE_TRANSPORT,
    TRACE_RWX,
    TRACE_RWX,       /* 0x010 */
    TRACE_USB,
    TRACE_SYNC,
    TRACE_SYSDEPS,
    TRACE_JDWP,
    TRACE_JDWP,      /* 0x100 */
    TRACE_SERVICES,
} AdbTrace;

#if ADB_TRACE

  int     adb_trace_mask;

  extern int     adb_trace_mask;
  extern unsigned char    adb_trace_output_count;
  void    adb_trace_init(void);

#  define ADB_TRACING  ((adb_trace_mask & (1 << TRACE_TAG)) != 0)
@@ -353,11 +349,33 @@ typedef enum {
  /* you must define TRACE_TAG before using this macro */
#  define  D(...)                                      \
        do {                                           \
            if (ADB_TRACING)                           \
            if (ADB_TRACING) {                         \
                int save_errno = errno;                \
                adb_mutex_lock(&D_lock);               \
                fprintf(stderr, "%s::%s():",           \
                        __FILE__, __FUNCTION__);       \
                errno = save_errno;                    \
                fprintf(stderr, __VA_ARGS__ );         \
                fflush(stderr);                        \
                adb_mutex_unlock(&D_lock);             \
                errno = save_errno;                    \
           }                                           \
        } while (0)
#  define  DR(...)                                     \
        do {                                           \
            if (ADB_TRACING) {                         \
                int save_errno = errno;                \
                adb_mutex_lock(&D_lock);               \
                errno = save_errno;                    \
                fprintf(stderr, __VA_ARGS__ );         \
                fflush(stderr);                        \
                adb_mutex_unlock(&D_lock);             \
                errno = save_errno;                    \
           }                                           \
        } while (0)
#else
#  define  D(...)          ((void)0)
#  define  DR(...)         ((void)0)
#  define  ADB_TRACING     0
#endif

@@ -413,6 +431,7 @@ int connection_state(atransport *t);
#define CS_NOPERM     5 /* Insufficient permissions to communicate with the device */

extern int HOST;
extern int SHELL_EXIT_NOTIFY_FD;

#define CHUNK_SIZE (64*1024)

+3 −0
Original line number Diff line number Diff line
@@ -202,6 +202,7 @@ int _adb_connect(const char *service)
        return -1;
    }

    D("_adb_connect: return fd %d\n", fd);
    return fd;
}

@@ -210,6 +211,7 @@ int adb_connect(const char *service)
    // first query the adb server's version
    int fd = _adb_connect("host:version");

    D("adb_connect: service %s\n", service);
    if(fd == -2) {
        fprintf(stdout,"* daemon not running. starting it now on port %d *\n",
                __adb_server_port);
@@ -266,6 +268,7 @@ int adb_connect(const char *service)
    if(fd == -2) {
        fprintf(stderr,"** daemon still not running");
    }
    D("adb_connect: return fd %d\n", fd);

    return fd;
error:
+9 −11
Original line number Diff line number Diff line
@@ -37,12 +37,6 @@
#include "adb_client.h"
#include "file_sync_service.h"

enum {
    IGNORE_DATA,
    WIPE_DATA,
    FLASH_DATA
};

static int do_cmd(transport_type ttype, char* serial, char *cmd, ...);

void get_my_path(char *s, size_t maxLen);
@@ -138,11 +132,6 @@ void help()
        "  adb help                     - show this help message\n"
        "  adb version                  - show version num\n"
        "\n"
        "DATAOPTS:\n"
        " (no option)                   - don't touch the data partition\n"
        "  -w                           - wipe the data partition\n"
        "  -d                           - flash the data partition\n"
        "\n"
        "scripting:\n"
        "  adb wait-for-device          - block until device is online\n"
        "  adb start-server             - ensure that there is a server running\n"
@@ -218,7 +207,9 @@ static void read_and_dump(int fd)
    int len;

    while(fd >= 0) {
        D("read_and_dump(): pre adb_read(fd=%d)\n", fd);
        len = adb_read(fd, buf, 4096);
        D("read_and_dump(): post adb_read(fd=%d): len=%d\n", fd, len);
        if(len == 0) {
            break;
        }
@@ -246,7 +237,9 @@ static void *stdin_read_thread(void *x)

    for(;;) {
        /* fdi is really the client's stdin, so use read, not adb_read here */
        D("stdin_read_thread(): pre unix_read(fdi=%d,...)\n", fdi);
        r = unix_read(fdi, buf, 1024);
        D("stdin_read_thread(): post unix_read(fdi=%d,...)\n", fdi);
        if(r == 0) break;
        if(r < 0) {
            if(errno == EINTR) continue;
@@ -853,6 +846,7 @@ top:
        }

        if(argc < 2) {
            D("starting interactive shell\n");
            r = interactive_shell();
            if (h) {
                printf("\x1b[0m");
@@ -877,9 +871,12 @@ top:
        }

        for(;;) {
            D("interactive shell loop. buff=%s\n", buf);
            fd = adb_connect(buf);
            if(fd >= 0) {
                D("about to read_and_dump(fd=%d)\n", fd);
                read_and_dump(fd);
                D("read_and_dump() done.\n");
                adb_close(fd);
                r = 0;
            } else {
@@ -896,6 +893,7 @@ top:
                    printf("\x1b[0m");
                    fflush(stdout);
                }
                D("interactive shell loop. return r=%d\n", r);
                return r;
            }
        }
+215 −26
Original line number Diff line number Diff line
@@ -15,6 +15,8 @@
** limitations under the License.
*/

#include <sys/ioctl.h>

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
@@ -27,10 +29,20 @@
#include <stddef.h>

#include "fdevent.h"
#include "transport.h"
#include "sysdeps.h"


#define TRACE(x...) fprintf(stderr,x)
/* !!! Do not enable DEBUG for the adb that will run as the server:
** both stdout and stderr are used to communicate between the client
** and server. Any extra output will cause failures.
*/
#define DEBUG 0   /* non-0 will break adb server */

#define DEBUG 0
// This socket is used when a subproc shell service exists.
// It wakes up the fdevent_loop() and cause the correct handling
// of the shell's pseudo-tty master. I.e. force close it.
int SHELL_EXIT_NOTIFY_FD = -1;

static void fatal(const char *fn, const char *fmt, ...)
{
@@ -45,15 +57,28 @@ static void fatal(const char *fn, const char *fmt, ...)
#define FATAL(x...) fatal(__FUNCTION__, x)

#if DEBUG
#define D(...) \
    do { \
        adb_mutex_lock(&D_lock);               \
        int save_errno = errno;                \
        fprintf(stderr, "%s::%s():", __FILE__, __FUNCTION__);  \
        errno = save_errno;                    \
        fprintf(stderr, __VA_ARGS__);          \
        adb_mutex_unlock(&D_lock);             \
        errno = save_errno;                    \
    } while(0)
static void dump_fde(fdevent *fde, const char *info)
{
    adb_mutex_lock(&D_lock);
    fprintf(stderr,"FDE #%03d %c%c%c %s\n", fde->fd,
            fde->state & FDE_READ ? 'R' : ' ',
            fde->state & FDE_WRITE ? 'W' : ' ',
            fde->state & FDE_ERROR ? 'E' : ' ',
            info);
    adb_mutex_unlock(&D_lock);
}
#else
#define D(...) ((void)0)
#define dump_fde(fde, info) do { } while(0)
#endif

@@ -67,6 +92,7 @@ static void dump_fde(fdevent *fde, const char *info)
static void fdevent_plist_enqueue(fdevent *node);
static void fdevent_plist_remove(fdevent *node);
static fdevent *fdevent_plist_dequeue(void);
static void fdevent_subproc_event_func(int fd, unsigned events, void *userdata);

static fdevent list_pending = {
    .next = &list_pending,
@@ -273,6 +299,69 @@ static void fdevent_update(fdevent *fde, unsigned events)
    fde->state = (fde->state & FDE_STATEMASK) | events;
}

/* Looks at fd_table[] for bad FDs and sets bit in fds.
** Returns the number of bad FDs.
*/
static int fdevent_fd_check(fd_set *fds)
{
    int i, n = 0;
    fdevent *fde;

    for(i = 0; i < select_n; i++) {
        fde = fd_table[i];
        if(fde == 0) continue;
        if(fcntl(i, F_GETFL, NULL) < 0) {
            FD_SET(i, fds);
            n++;
            // fde->state |= FDE_DONT_CLOSE;

        }
    }
    return n;
}

#if !DEBUG
static inline void dump_all_fds(const char *extra_msg) {}
#else
static void dump_all_fds(const char *extra_msg)
{
int i;
    fdevent *fde;
    // per fd: 4 digits (but really: log10(FD_SETSIZE)), 1 staus, 1 blank
    char msg_buff[FD_SETSIZE*6 + 1], *pb=msg_buff;
    size_t max_chars = FD_SETSIZE * 6 + 1;
    int printed_out;
#define SAFE_SPRINTF(...)                                                    \
    do {                                                                     \
        printed_out = snprintf(pb, max_chars, __VA_ARGS__);                  \
        if (printed_out <= 0) {                                              \
            D("... snprintf failed.\n");                                     \
            return;                                                          \
        }                                                                    \
        if (max_chars < (unsigned int)printed_out) {                         \
            D("... snprintf out of space.\n");                               \
            return;                                                          \
        }                                                                    \
        pb += printed_out;                                                   \
        max_chars -= printed_out;                                            \
    } while(0)

    for(i = 0; i < select_n; i++) {
        fde = fd_table[i];
        SAFE_SPRINTF("%d", i);
        if(fde == 0) {
            SAFE_SPRINTF("? ");
            continue;
        }
        if(fcntl(i, F_GETFL, NULL) < 0) {
            SAFE_SPRINTF("b");
        }
        SAFE_SPRINTF(" ");
    }
    D("%s fd_table[]->fd = {%s}\n", extra_msg, msg_buff);
}
#endif

static void fdevent_process()
{
    int i, n;
@@ -284,28 +373,49 @@ static void fdevent_process()
    memcpy(&wfd, &write_fds, sizeof(fd_set));
    memcpy(&efd, &error_fds, sizeof(fd_set));

    n = select(select_n, &rfd, &wfd, &efd, 0);
    dump_all_fds("pre select()");

    n = select(select_n, &rfd, &wfd, &efd, NULL);
    int saved_errno = errno;
    D("select() returned n=%d, errno=%d\n", n, n<0?saved_errno:0);

    dump_all_fds("post select()");

    if(n < 0) {
        if(errno == EINTR) return;
        perror("select");
        switch(saved_errno) {
        case EINTR: return;
        case EBADF:
            // Can't trust the FD sets after an error.
            FD_ZERO(&wfd);
            FD_ZERO(&efd);
            FD_ZERO(&rfd);
            break;
        default:
            D("Unexpected select() error=%d\n", saved_errno);
            return;
        }
    }
    if(n <= 0) {
        // We fake a read, as the rest of the code assumes
        // that errors will be detected at that point.
        n = fdevent_fd_check(&rfd);
    }

    for(i = 0; (i < select_n) && (n > 0); i++) {
        events = 0;
        if(FD_ISSET(i, &rfd)) events |= FDE_READ;
        if(FD_ISSET(i, &wfd)) events |= FDE_WRITE;
        if(FD_ISSET(i, &efd)) events |= FDE_ERROR;
        if(FD_ISSET(i, &rfd)) { events |= FDE_READ; n--; }
        if(FD_ISSET(i, &wfd)) { events |= FDE_WRITE; n--; }
        if(FD_ISSET(i, &efd)) { events |= FDE_ERROR; n--; }

        if(events) {
            n--;

            fde = fd_table[i];
            if(fde == 0) FATAL("missing fde for fd %d\n", i);
            if(fde == 0)
              FATAL("missing fde for fd %d\n", i);

            fde->events |= events;

            D("got events fde->fd=%d events=%04x, state=%04x\n",
                fde->fd, fde->events, fde->state);
            if(fde->state & FDE_PENDING) continue;
            fde->state |= FDE_PENDING;
            fdevent_plist_enqueue(fde);
@@ -350,14 +460,14 @@ static void fdevent_unregister(fdevent *fde)
    }

    if(fd_table[fde->fd] != fde) {
        FATAL("fd_table out of sync");
        FATAL("fd_table out of sync [%d]\n", fde->fd);
    }

    fd_table[fde->fd] = 0;

    if(!(fde->state & FDE_DONT_CLOSE)) {
        dump_fde(fde, "close");
        close(fde->fd);
        adb_close(fde->fd);
    }
}

@@ -394,6 +504,74 @@ static fdevent *fdevent_plist_dequeue(void)
    return node;
}

static void fdevent_call_fdfunc(fdevent* fde)
{
    unsigned events = fde->events;
    fde->events = 0;
    if(!(fde->state & FDE_PENDING)) return;
    fde->state &= (~FDE_PENDING);
    dump_fde(fde, "callback");
    fde->func(fde->fd, events, fde->arg);
}

static void fdevent_subproc_event_func(int fd, unsigned ev, void *userdata)
{

    D("subproc handling on fd=%d ev=%04x\n", fd, ev);

    // Hook oneself back into the fde's suitable for select() on read.
    if((fd < 0) || (fd >= fd_table_max)) {
        FATAL("fd %d out of range for fd_table \n", fd);
    }
    fdevent *fde = fd_table[fd];
    fdevent_add(fde, FDE_READ);

    if(ev & FDE_READ){
      int subproc_fd;

      if(readx(fd, &subproc_fd, sizeof(subproc_fd))) {
          FATAL("Failed to read the subproc's fd from fd=%d\n", fd);
      }
      if((subproc_fd < 0) || (subproc_fd >= fd_table_max)) {
          D("subproc_fd %d out of range 0, fd_table_max=%d\n",
            subproc_fd, fd_table_max);
          return;
      }
      fdevent *subproc_fde = fd_table[subproc_fd];
      if(!subproc_fde) {
          D("subproc_fd %d cleared from fd_table\n", subproc_fd);
          return;
      }
      if(subproc_fde->fd != subproc_fd) {
          // Already reallocated?
          D("subproc_fd %d != fd_table[].fd %d\n", subproc_fd, subproc_fde->fd);
          return;
      }

      subproc_fde->force_eof = 1;

      int rcount = 0;
      ioctl(subproc_fd, TIOCINQ, &rcount);
      D("subproc with fd=%d  has rcount=%d err=%d\n",
        subproc_fd, rcount, errno);

      if(rcount) {
        // If there is data left, it will show up in the select().
        // This works because there is no other thread reading that
        // data when in this fd_func().
        return;
      }

      D("subproc_fde.state=%04x\n", subproc_fde->state);
      subproc_fde->events |= FDE_READ;
      if(subproc_fde->state & FDE_PENDING) {
        return;
      }
      subproc_fde->state |= FDE_PENDING;
      fdevent_call_fdfunc(subproc_fde);
    }
}

fdevent *fdevent_create(int fd, fd_func func, void *arg)
{
    fdevent *fde = (fdevent*) malloc(sizeof(fdevent));
@@ -417,6 +595,7 @@ void fdevent_install(fdevent *fde, int fd, fd_func func, void *arg)
    memset(fde, 0, sizeof(fdevent));
    fde->state = FDE_ACTIVE;
    fde->fd = fd;
    fde->force_eof = 0;
    fde->func = func;
    fde->arg = arg;

@@ -484,23 +663,33 @@ void fdevent_del(fdevent *fde, unsigned events)
        fde, (fde->state & FDE_EVENTMASK) & (~(events & FDE_EVENTMASK)));
}

void fdevent_subproc_setup()
{
    int s[2];

    if(adb_socketpair(s)) {
        FATAL("cannot create shell-exit socket-pair\n");
    }
    SHELL_EXIT_NOTIFY_FD = s[0];
    fdevent *fde;
    fde = fdevent_create(s[1], fdevent_subproc_event_func, NULL);
    if(!fde)
      FATAL("cannot create fdevent for shell-exit handler\n");
    fdevent_add(fde, FDE_READ);
}

void fdevent_loop()
{
    fdevent *fde;
    fdevent_subproc_setup();

    for(;;) {
#if DEBUG
        fprintf(stderr,"--- ---- waiting for events\n");
#endif
        D("--- ---- waiting for events\n");

        fdevent_process();

        while((fde = fdevent_plist_dequeue())) {
            unsigned events = fde->events;
            fde->events = 0;
            fde->state &= (~FDE_PENDING);
            dump_fde(fde, "callback");
            fde->func(fde->fd, events, fde->arg);
            fdevent_call_fdfunc(fde);
        }
    }
}
Loading