From c126e4079575d2c32b2b74508739a5de7b27ec89 Mon Sep 17 00:00:00 2001 From: Gao feng Date: Fri, 23 Oct 2015 16:40:10 +0800 Subject: [PATCH] use reference to protect exec structure stdout, stderr and signal loop will try to access exec struct. right now, we don't have reference of exec for stderr handler. so in stderr epoll handler, it may access to the already released exec resource. Signed-off-by: Gao feng --- src/container.c | 19 ++++++++----- src/exec.c | 76 ++++++++++++++++++++++++++++++------------------- src/exec.h | 1 + src/init.c | 1 - src/parse.c | 1 + 5 files changed, 61 insertions(+), 37 deletions(-) diff --git a/src/container.c b/src/container.c index cecc51f..ec7f167 100644 --- a/src/container.c +++ b/src/container.c @@ -505,6 +505,11 @@ int hyper_start_container(struct hyper_container *container, goto fail; } + if (hyper_watch_exec_pty(&container->exec, pod) < 0) { + fprintf(stderr, "faile to watch container pty\n"); + goto fail; + } + stack = malloc(stacksize); if (stack == NULL) { perror("fail to allocate stack for container init"); @@ -518,28 +523,27 @@ int hyper_start_container(struct hyper_container *container, goto fail; } sprintf(path, "/proc/%d/ns/mnt", pid); - container->exec.pid = pid; + container->ns = open(path, O_RDONLY | O_CLOEXEC); if (container->ns < 0) { perror("open container mount ns failed"); goto fail; } - if (hyper_watch_exec_pty(&container->exec, pod) < 0) { - fprintf(stderr, "faile to watch container pty\n"); - goto fail; - } - /* wait for ready message */ if (hyper_get_type(arg.pipe[0], &type) < 0 || type != READY) { fprintf(stderr, "wait for container started failed\n"); goto fail; } + container->exec.pid = pid; + list_add_tail(&container->exec.list, &pod->exec_head); + container->exec.ref++; + close(arg.pipe[0]); close(arg.pipe[1]); - fprintf(stdout, "container %s init pid is %d\n", container->id, pid); + fprintf(stdout, "container %s,init pid %d,ref %d\n", container->id, pid, container->exec.ref); return 0; fail: close(arg.pipe[0]); @@ -551,6 +555,7 @@ fail: fprintf(stdout, "container %s init exit code %d\n", container->id, -1); container->exec.code = -1; container->exec.seq = 0; + container->exec.ref = 0; return -1; } diff --git a/src/exec.c b/src/exec.c index 3916134..70d4215 100644 --- a/src/exec.c +++ b/src/exec.c @@ -18,11 +18,9 @@ #include "util.h" #include "parse.h" -static void pts_hup(struct hyper_event *de, int efd) +static void pts_hup(struct hyper_exec *exec, struct hyper_pod *pod, uint64_t seq) { - struct hyper_pod *pod = de->ptr; struct hyper_buf *buf = &ctl.tty.wbuf; - struct hyper_exec *exec = container_of(de, struct hyper_exec, e); fprintf(stdout, "%s\n", __func__); @@ -32,24 +30,35 @@ static void pts_hup(struct hyper_event *de, int efd) } /* no in event, no more data, send eof */ - hyper_set_be64(buf->data + buf->get, exec->seq); + hyper_set_be64(buf->data + buf->get, seq); hyper_set_be32(buf->data + buf->get + 8, 12); buf->get += 12; - if (buf->get + 12 > buf->size) { - fprintf(stdout, "%s: tty buf full (for stderr)\n", __func__); - } else { - /* no in event, no more data, send eof in stderr */ - hyper_set_be64(buf->data + buf->get, exec->errseq); - hyper_set_be32(buf->data + buf->get + 8, 12); - buf->get += 12; - } - hyper_modify_event(ctl.efd, &ctl.tty, EPOLLIN | EPOLLOUT); hyper_release_exec(exec, pod); } +static void stdout_hup(struct hyper_event *de, int efd) +{ + struct hyper_exec *exec = container_of(de, struct hyper_exec, e); + struct hyper_pod *pod = de->ptr; + + fprintf(stdout, "%s, seq %" PRIu64"\n", __func__, exec->seq); + + return pts_hup(exec, pod, exec->seq); +} + +static void stderr_hup(struct hyper_event *de, int efd) +{ + struct hyper_exec *exec = container_of(de, struct hyper_exec, errev); + struct hyper_pod *pod = de->ptr; + + fprintf(stdout, "%s, seq %" PRIu64"\n", __func__, exec->errseq); + + return pts_hup(exec, pod, exec->errseq); +} + static int pts_loop(struct hyper_event *de, uint64_t seq) { int size = -1; @@ -93,7 +102,7 @@ static int stdout_loop(struct hyper_event *de) struct hyper_event_ops pts_ops = { .read = stdout_loop, - .hup = pts_hup, + .hup = stdout_hup, .write = hyper_event_write, .wbuf_size = 512, /* don't need read buff, the pts data will store in tty buffer */ @@ -108,8 +117,8 @@ static int stderr_loop(struct hyper_event *de) } struct hyper_event_ops err_ops = { - /* don't need to deal with hup, the hup will be dealed by pts*/ .read = stderr_loop, + .hup = stderr_hup, /* don't need read buff, the stderr data will store in tty buffer */ /* don't need write buff, the stderr data is one way */ }; @@ -173,6 +182,8 @@ int hyper_setup_exec_tty(struct hyper_exec *e) e->ptyfd = open(ptmx, O_RDWR | O_NOCTTY | O_CLOEXEC); fprintf(stdout, "get pty device for exec %s\n", ptmx); + fprintf(stdout, "%s pts event %p, fd %d %d\n", + __func__, &e->e, e->e.fd, e->ptyfd); return 0; } @@ -249,16 +260,17 @@ int hyper_watch_exec_pty(struct hyper_exec *exec, struct hyper_pod *pod) fprintf(stderr, "add container pts master event failed\n"); return -1; } + exec->ref++; if (exec->errseq == 0) return 0; - if (hyper_init_event(&exec->errev, &err_ops, NULL) < 0 || + if (hyper_init_event(&exec->errev, &err_ops, pod) < 0 || hyper_add_event(ctl.efd, &exec->errev, EPOLLIN) < 0) { fprintf(stderr, "add container stderr event failed\n"); return -1; } - + exec->ref++; return 0; } @@ -363,6 +375,11 @@ static int hyper_do_exec_cmd(void *data) goto out; } + if (hyper_watch_exec_pty(exec, pod) < 0) { + fprintf(stderr, "add pts master event failed\n"); + goto out; + } + pid = fork(); if (pid < 0) { perror("fail to fork"); @@ -370,11 +387,6 @@ static int hyper_do_exec_cmd(void *data) } else if (pid > 0) { uint32_t type; - if (hyper_watch_exec_pty(exec, pod) < 0) { - fprintf(stderr, "add pts master event failed\n"); - goto out; - } - if (hyper_get_type(pipe[0], &type) < 0 || type != READY) { fprintf(stderr, "hyper init doesn't get execcmd ready message\n"); hyper_send_type(arg->pipe[1], ERROR); @@ -383,9 +395,10 @@ static int hyper_do_exec_cmd(void *data) fprintf(stdout, "hyper init get ready message\n"); exec->pid = pid; - fprintf(stdout, "create exec cmd %s pid %d\n", exec->argv[0], pid); - + //TODO combin ref++ and add to list. list_add_tail(&exec->list, &pod->exec_head); + exec->ref++; + fprintf(stdout, "create exec cmd %s pid %d,ref %d\n", exec->argv[0], pid, exec->ref); ret = 0; goto out; } @@ -499,6 +512,7 @@ out: return ret; close_tty: close(exec->ptyfd); + close(exec->errfd); close(exec->e.fd); free_exec: hyper_free_exec(exec); @@ -508,11 +522,8 @@ free_exec: int hyper_release_exec(struct hyper_exec *exec, struct hyper_pod *pod) { - if (!exec->exit && exec->seq) { - fprintf(stdout, "first user of exec exit\n"); - exec->exit = 1; - close(exec->ptyfd); - exec->ptyfd = -1; + if (--exec->ref != 0) { + fprintf(stdout, "still have %d user of exec\n", exec->ref); return 0; } @@ -614,6 +625,13 @@ int hyper_send_exec_eof(int to, struct hyper_pod *pod, __func__, exec->pid, exec->seq, exec->id ? exec->id : "pod"); exec->code = code; + exec->exit = 1; + + close(exec->ptyfd); + exec->ptyfd = -1; + close(exec->errfd); + exec->errfd = -1; + hyper_release_exec(exec, pod); return 0; diff --git a/src/exec.h b/src/exec.h index 2726e08..2da0dd8 100644 --- a/src/exec.h +++ b/src/exec.h @@ -20,6 +20,7 @@ struct hyper_exec { int errfd; uint8_t code; uint8_t exit; + uint8_t ref; }; struct hyper_pod; diff --git a/src/init.c b/src/init.c index 56bdf4a..fd81cee 100644 --- a/src/init.c +++ b/src/init.c @@ -396,7 +396,6 @@ static int hyper_do_start_containers(void *data) fprintf(stderr, "fail to start container\n"); goto out; } - list_add_tail(&c->exec.list, &pod->exec_head); } ret = 0; diff --git a/src/parse.c b/src/parse.c index a159b31..040016b 100644 --- a/src/parse.c +++ b/src/parse.c @@ -615,6 +615,7 @@ realloc: goto out; exec->ptyfd = -1; + exec->errfd = -1; exec->e.fd = -1; INIT_LIST_HEAD(&exec->list);