From 5e509aad414f85f686bc07e9f8962b40b144f5ef Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Sun, 2 Dec 2012 15:06:00 +0100 Subject: [PATCH] eloop: add child-reaper event sources If multiple childs die simultaneously, only a single SIGCHLD is queued. Only after our process got scheduled and we unqueued the SIGCHLD, a next signal can be queued. Therefore, using SIGCHLD as indicator that a specific child died does not work reliably. Instead, we must use waitpid() to wait for childs. Hence, we introduce a new event-source which does that. It reports the PID/status combination for each child to the registered callbacks. Signed-off-by: David Herrmann --- src/eloop.c | 83 ++++++++++++++++++++++++++++++++++++++++++++--------- src/eloop.h | 32 +++++++++++++++++++++ 2 files changed, 102 insertions(+), 13 deletions(-) diff --git a/src/eloop.c b/src/eloop.c index 307efcd..55e8136 100644 --- a/src/eloop.c +++ b/src/eloop.c @@ -205,6 +205,7 @@ struct ev_eloop { int idle_fd; struct shl_dlist sig_list; + struct shl_hook *chlds; struct shl_hook *idlers; struct shl_hook *pres; struct shl_hook *posts; @@ -317,36 +318,38 @@ struct ev_signal_shared { * thread when a signalfd is created. We never unblock the signal. However, * most modern linux user-space programs avoid signal handlers, anyway, so you * can use signalfd only. - * - * As special note, we automatically handle SIGCHLD signals here and wait for - * all pending child exits. This, however, is only activated when at least one - * user has registered for SIGCHLD callbacks. */ -static void sig_child(struct ev_fd *fd) +static void sig_child(struct ev_eloop *loop, struct signalfd_siginfo *info, + void *data) { pid_t pid; int status; + struct ev_child_data d; while (1) { pid = waitpid(-1, &status, WNOHANG); if (pid == -1) { if (errno != ECHILD) - llog_warn(fd, "cannot wait on child: %m"); + llog_warn(loop, "cannot wait on child: %m"); break; } else if (pid == 0) { break; } else if (WIFEXITED(status)) { if (WEXITSTATUS(status) != 0) - llog_debug(fd, "child %d exited with status %d", + llog_debug(loop, "child %d exited with status %d", pid, WEXITSTATUS(status)); else - llog_debug(fd, "child %d exited successfully", + llog_debug(loop, "child %d exited successfully", pid); } else if (WIFSIGNALED(status)) { - llog_debug(fd, "child %d exited by signal %d", pid, + llog_debug(loop, "child %d exited by signal %d", pid, WTERMSIG(status)); } + + d.pid = pid; + d.status = status; + shl_hook_call(loop->chlds, loop, &d); } } @@ -362,9 +365,6 @@ static void shared_signal_cb(struct ev_fd *fd, int mask, void *data) llog_warn(fd, "cannot read signalfd (%d): %m", errno); else shl_hook_call(sig->hook, sig->fd->loop, &info); - - if (info.ssi_signo == SIGCHLD) - sig_child(fd); } else if (mask & (EV_HUP | EV_ERR)) { llog_warn(fd, "HUP/ERR on signal source"); } @@ -603,10 +603,14 @@ int ev_eloop_new(struct ev_eloop **out, ev_log_t log) goto err_free; } - ret = shl_hook_new(&loop->idlers); + ret = shl_hook_new(&loop->chlds); if (ret) goto err_fds; + ret = shl_hook_new(&loop->idlers); + if (ret) + goto err_childs; + ret = shl_hook_new(&loop->pres); if (ret) goto err_idlers; @@ -662,6 +666,8 @@ err_pres: shl_hook_free(loop->pres); err_idlers: shl_hook_free(loop->idlers); +err_childs: + shl_hook_free(loop->chlds); err_fds: free(loop->cur_fds); err_free: @@ -706,6 +712,9 @@ void ev_eloop_unref(struct ev_eloop *loop) llog_debug(loop, "free eloop object %p", loop); + if (shl_hook_num(loop->chlds)) + ev_eloop_unregister_signal_cb(loop, SIGCHLD, sig_child, loop); + while (loop->sig_list.next != &loop->sig_list) { sig = shl_dlist_entry(loop->sig_list.next, struct ev_signal_shared, @@ -724,6 +733,7 @@ void ev_eloop_unref(struct ev_eloop *loop) shl_hook_free(loop->posts); shl_hook_free(loop->pres); shl_hook_free(loop->idlers); + shl_hook_free(loop->chlds); free(loop->cur_fds); free(loop); } @@ -2227,6 +2237,53 @@ void ev_eloop_unregister_signal_cb(struct ev_eloop *loop, int signum, } } +/* + * Child reaper sources + * If at least one child-reaper callback is registered, then the eloop object + * listens for SIGCHLD and waits for all exiting children. The callbacks are + * then notified for each PID that signaled an event. + * Note that this cannot be done via the shared-signal sources as the waitpid() + * call must not be done in callbacks. Otherwise, only one callback would see + * the events while others will call waitpid() and get EAGAIN. + */ + +int ev_eloop_register_child_cb(struct ev_eloop *loop, ev_child_cb cb, + void *data) +{ + int ret; + bool empty; + + if (!loop) + return -EINVAL; + + empty = !shl_hook_num(loop->chlds); + ret = shl_hook_add_cast(loop->chlds, cb, data); + if (ret) + return ret; + + if (empty) { + ret = ev_eloop_register_signal_cb(loop, SIGCHLD, sig_child, + loop); + if (ret) { + shl_hook_rm_cast(loop->chlds, cb, data); + return ret; + } + } + + return 0; +} + +void ev_eloop_unregister_child_cb(struct ev_eloop *loop, ev_child_cb cb, + void *data) +{ + if (!loop || !shl_hook_num(loop->chlds)) + return; + + shl_hook_rm_cast(loop->chlds, cb, data); + if (!shl_hook_num(loop->chlds)) + ev_eloop_unregister_signal_cb(loop, SIGCHLD, sig_child, loop); +} + /* * Idle sources * Idle sources are called everytime when a next dispatch round is started. diff --git a/src/eloop.h b/src/eloop.h index c2e608a..c984dc9 100644 --- a/src/eloop.h +++ b/src/eloop.h @@ -39,6 +39,7 @@ #include #include #include +#include #include struct ev_eloop; @@ -113,6 +114,30 @@ typedef void (*ev_counter_cb) typedef void (*ev_signal_shared_cb) (struct ev_eloop *eloop, struct signalfd_siginfo *info, void *data); +/** + * ev_child_data: + * @pid: pid of child + * @status: status of child + * + * This contains the payload for @ev_child_cb events. The data is simply copied + * from the waitpid() system-call. + */ +struct ev_child_data { + pid_t pid; + int status; +}; + +/** + * ev_child_cb: + * @eloop: event loop object + * @chld: chld pid/status payload + * @data: user-supplied data + * + * This is the callback-type for child-reaper events. + */ +typedef void (*ev_child_cb) + (struct ev_eloop *eloop, struct ev_child_data *chld, void *data); + /** * ev_idle_cb: * @eloop: event loop object @@ -228,6 +253,13 @@ int ev_eloop_register_signal_cb(struct ev_eloop *loop, int signum, void ev_eloop_unregister_signal_cb(struct ev_eloop *loop, int signum, ev_signal_shared_cb cb, void *data); +/* child reaper sources */ + +int ev_eloop_register_child_cb(struct ev_eloop *loop, ev_child_cb cb, + void *data); +void ev_eloop_unregister_child_cb(struct ev_eloop *loop, ev_child_cb cb, + void *data); + /* idle sources */ int ev_eloop_register_idle_cb(struct ev_eloop *eloop, ev_idle_cb cb,