From 45616f542da523b3d87a8d379bb116ffc3a97845 Mon Sep 17 00:00:00 2001 From: Santiago Gimeno Date: Sun, 23 Oct 2016 14:47:31 +0200 Subject: [PATCH] signal: add uv_signal_start_oneshot method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It behaves as `uv_signal_start` but it resets the signal handler as soon as the signal is received. Fixes: https://github.com/libuv/libuv/issues/1104 PR-URL: https://github.com/libuv/libuv/pull/1106 Reviewed-By: Ben Noordhuis Reviewed-By: Saúl Ibarra Corretgé Reviewed-By: Bartosz Sosnowski --- docs/src/signal.rst | 7 +++ include/uv.h | 3 + src/unix/signal.c | 63 +++++++++++++++++-- src/uv-common.h | 11 ++-- src/win/signal.c | 35 ++++++++++- test/test-list.h | 4 ++ test/test-signal.c | 146 +++++++++++++++++++++++++++++++++++++++++--- 7 files changed, 250 insertions(+), 19 deletions(-) diff --git a/docs/src/signal.rst b/docs/src/signal.rst index dc1223b9..5b3b352b 100644 --- a/docs/src/signal.rst +++ b/docs/src/signal.rst @@ -70,6 +70,13 @@ API Start the handle with the given callback, watching for the given signal. +.. c:function:: int uv_signal_start_oneshot(uv_signal_t* signal, uv_signal_cb cb, int signum) + + .. versionadded:: 1.12.0 + + Same functionality as :c:func:`uv_signal_start` but the signal handler is reset the moment + the signal is received. + .. c:function:: int uv_signal_stop(uv_signal_t* signal) Stop the handle, the callback will no longer be called. diff --git a/include/uv.h b/include/uv.h index 31f09f0f..e5230bb5 100644 --- a/include/uv.h +++ b/include/uv.h @@ -1324,6 +1324,9 @@ UV_EXTERN int uv_signal_init(uv_loop_t* loop, uv_signal_t* handle); UV_EXTERN int uv_signal_start(uv_signal_t* handle, uv_signal_cb signal_cb, int signum); +UV_EXTERN int uv_signal_start_oneshot(uv_signal_t* handle, + uv_signal_cb signal_cb, + int signum); UV_EXTERN int uv_signal_stop(uv_signal_t* handle); UV_EXTERN void uv_loadavg(double avg[3]); diff --git a/src/unix/signal.c b/src/unix/signal.c index dbd8f864..63032dcb 100644 --- a/src/unix/signal.c +++ b/src/unix/signal.c @@ -38,9 +38,14 @@ RB_HEAD(uv__signal_tree_s, uv_signal_s); static int uv__signal_unlock(void); +static int uv__signal_start(uv_signal_t* handle, + uv_signal_cb signal_cb, + int signum, + int oneshot); static void uv__signal_event(uv_loop_t* loop, uv__io_t* w, unsigned int events); static int uv__signal_compare(uv_signal_t* w1, uv_signal_t* w2); static void uv__signal_stop(uv_signal_t* handle); +static void uv__signal_unregister_handler(int signum); static uv_once_t uv__signal_global_init_guard = UV_ONCE_INIT; @@ -122,6 +127,7 @@ static uv_signal_t* uv__signal_first_handle(int signum) { uv_signal_t* handle; lookup.signum = signum; + lookup.flags = 0; lookup.loop = NULL; handle = RB_NFIND(uv__signal_tree_s, &uv__signal_tree, &lookup); @@ -174,7 +180,7 @@ static void uv__signal_handler(int signum) { } -static int uv__signal_register_handler(int signum) { +static int uv__signal_register_handler(int signum, int oneshot) { /* When this function is called, the signal lock must be held. */ struct sigaction sa; @@ -183,6 +189,7 @@ static int uv__signal_register_handler(int signum) { if (sigfillset(&sa.sa_mask)) abort(); sa.sa_handler = uv__signal_handler; + sa.sa_flags = oneshot ? SA_RESETHAND : 0; /* XXX save old action so we can restore it later on? */ if (sigaction(signum, &sa, NULL)) @@ -287,8 +294,24 @@ void uv__signal_close(uv_signal_t* handle) { int uv_signal_start(uv_signal_t* handle, uv_signal_cb signal_cb, int signum) { + return uv__signal_start(handle, signal_cb, signum, 0); +} + + +int uv_signal_start_oneshot(uv_signal_t* handle, + uv_signal_cb signal_cb, + int signum) { + return uv__signal_start(handle, signal_cb, signum, 1); +} + + +static int uv__signal_start(uv_signal_t* handle, + uv_signal_cb signal_cb, + int signum, + int oneshot) { sigset_t saved_sigmask; int err; + uv_signal_t* first_handle; assert(!uv__is_closing(handle)); @@ -318,9 +341,12 @@ int uv_signal_start(uv_signal_t* handle, uv_signal_cb signal_cb, int signum) { /* If at this point there are no active signal watchers for this signum (in * any of the loops), it's time to try and register a handler for it here. + * Also in case there's only one-shot handlers and a regular handler comes in. */ - if (uv__signal_first_handle(signum) == NULL) { - err = uv__signal_register_handler(signum); + first_handle = uv__signal_first_handle(signum); + if (first_handle == NULL || + (!oneshot && (first_handle->flags & UV__SIGNAL_ONE_SHOT))) { + err = uv__signal_register_handler(signum, oneshot); if (err) { /* Registering the signal handler failed. Must be an invalid signal. */ uv__signal_unlock_and_unblock(&saved_sigmask); @@ -329,6 +355,9 @@ int uv_signal_start(uv_signal_t* handle, uv_signal_cb signal_cb, int signum) { } handle->signum = signum; + if (oneshot) + handle->flags |= UV__SIGNAL_ONE_SHOT; + RB_INSERT(uv__signal_tree_s, &uv__signal_tree, handle); uv__signal_unlock_and_unblock(&saved_sigmask); @@ -390,6 +419,9 @@ static void uv__signal_event(uv_loop_t* loop, handle->dispatched_signals++; + if (handle->flags & UV__SIGNAL_ONE_SHOT) + uv__signal_stop(handle); + /* If uv_close was called while there were caught signals that were not * yet dispatched, the uv__finish_close was deferred. Make close pending * now if this has happened. @@ -414,12 +446,22 @@ static void uv__signal_event(uv_loop_t* loop, static int uv__signal_compare(uv_signal_t* w1, uv_signal_t* w2) { + int f1; + int f2; /* Compare signums first so all watchers with the same signnum end up * adjacent. */ if (w1->signum < w2->signum) return -1; if (w1->signum > w2->signum) return 1; + /* Handlers without UV__SIGNAL_ONE_SHOT set will come first, so if the first + * handler returned is a one-shot handler, the rest will be too. + */ + f1 = w1->flags & UV__SIGNAL_ONE_SHOT; + f2 = w2->flags & UV__SIGNAL_ONE_SHOT; + if (f1 < f2) return -1; + if (f1 > f2) return 1; + /* Sort by loop pointer, so we can easily look up the first item after * { .signum = x, .loop = NULL }. */ @@ -443,6 +485,10 @@ int uv_signal_stop(uv_signal_t* handle) { static void uv__signal_stop(uv_signal_t* handle) { uv_signal_t* removed_handle; sigset_t saved_sigmask; + uv_signal_t* first_handle; + int rem_oneshot; + int first_oneshot; + int ret; /* If the watcher wasn't started, this is a no-op. */ if (handle->signum == 0) @@ -457,8 +503,17 @@ static void uv__signal_stop(uv_signal_t* handle) { /* Check if there are other active signal watchers observing this signal. If * not, unregister the signal handler. */ - if (uv__signal_first_handle(handle->signum) == NULL) + first_handle = uv__signal_first_handle(handle->signum); + if (first_handle == NULL) { uv__signal_unregister_handler(handle->signum); + } else { + rem_oneshot = handle->flags & UV__SIGNAL_ONE_SHOT; + first_oneshot = first_handle->flags & UV__SIGNAL_ONE_SHOT; + if (first_oneshot && !rem_oneshot) { + ret = uv__signal_register_handler(handle->signum, 1); + assert(ret == 0); + } + } uv__signal_unlock_and_unblock(&saved_sigmask); diff --git a/src/uv-common.h b/src/uv-common.h index 27902fdf..b2b98e3b 100644 --- a/src/uv-common.h +++ b/src/uv-common.h @@ -55,16 +55,19 @@ extern int snprintf(char*, size_t, const char*, ...); #ifndef _WIN32 enum { + UV__SIGNAL_ONE_SHOT = 0x80000, /* On signal reception remove sighandler */ UV__HANDLE_INTERNAL = 0x8000, UV__HANDLE_ACTIVE = 0x4000, UV__HANDLE_REF = 0x2000, UV__HANDLE_CLOSING = 0 /* no-op on unix */ }; #else -# define UV__HANDLE_INTERNAL 0x80 -# define UV__HANDLE_ACTIVE 0x40 -# define UV__HANDLE_REF 0x20 -# define UV__HANDLE_CLOSING 0x01 +# define UV__SIGNAL_ONE_SHOT_DISPATCHED 0x200 +# define UV__SIGNAL_ONE_SHOT 0x100 +# define UV__HANDLE_INTERNAL 0x80 +# define UV__HANDLE_ACTIVE 0x40 +# define UV__HANDLE_REF 0x20 +# define UV__HANDLE_CLOSING 0x01 #endif int uv__loop_configure(uv_loop_t* loop, uv_loop_option option, va_list ap); diff --git a/src/win/signal.c b/src/win/signal.c index af7974c3..571563cb 100644 --- a/src/win/signal.c +++ b/src/win/signal.c @@ -34,6 +34,11 @@ static CRITICAL_SECTION uv__signal_lock; static BOOL WINAPI uv__signal_control_handler(DWORD type); +int uv__signal_start(uv_signal_t* handle, + uv_signal_cb signal_cb, + int signum, + int oneshot); + void uv_signals_init() { InitializeCriticalSection(&uv__signal_lock); if (!SetConsoleCtrlHandler(uv__signal_control_handler, TRUE)) @@ -70,7 +75,9 @@ RB_GENERATE_STATIC(uv_signal_tree_s, uv_signal_s, tree_entry, uv__signal_compare int uv__signal_dispatch(int signum) { uv_signal_t lookup; uv_signal_t* handle; - int dispatched = 0; + int dispatched; + + dispatched = 0; EnterCriticalSection(&uv__signal_lock); @@ -83,11 +90,16 @@ int uv__signal_dispatch(int signum) { unsigned long previous = InterlockedExchange( (volatile LONG*) &handle->pending_signum, signum); + if (handle->flags & UV__SIGNAL_ONE_SHOT_DISPATCHED) + continue; + if (!previous) { POST_COMPLETION_FOR_REQ(handle->loop, &handle->signal_req); } dispatched = 1; + if (handle->flags & UV__SIGNAL_ONE_SHOT) + handle->flags |= UV__SIGNAL_ONE_SHOT_DISPATCHED; } LeaveCriticalSection(&uv__signal_lock); @@ -166,6 +178,21 @@ int uv_signal_stop(uv_signal_t* handle) { int uv_signal_start(uv_signal_t* handle, uv_signal_cb signal_cb, int signum) { + return uv__signal_start(handle, signal_cb, signum, 0); +} + + +int uv_signal_start_oneshot(uv_signal_t* handle, + uv_signal_cb signal_cb, + int signum) { + return uv__signal_start(handle, signal_cb, signum, 1); +} + + +int uv__signal_start(uv_signal_t* handle, + uv_signal_cb signal_cb, + int signum, + int oneshot) { /* Test for invalid signal values. */ if (signum != SIGWINCH && (signum <= 0 || signum >= NSIG)) return UV_EINVAL; @@ -189,6 +216,9 @@ int uv_signal_start(uv_signal_t* handle, uv_signal_cb signal_cb, int signum) { EnterCriticalSection(&uv__signal_lock); handle->signum = signum; + if (oneshot) + handle->flags |= UV__SIGNAL_ONE_SHOT; + RB_INSERT(uv_signal_tree_s, &uv__signal_tree, handle); LeaveCriticalSection(&uv__signal_lock); @@ -217,6 +247,9 @@ void uv_process_signal_req(uv_loop_t* loop, uv_signal_t* handle, if (dispatched_signum == handle->signum) handle->signal_cb(handle, dispatched_signum); + if (handle->flags & UV__SIGNAL_ONE_SHOT) + uv_signal_stop(handle); + if (handle->flags & UV__HANDLE_CLOSING) { /* When it is closing, it must be stopped at this point. */ assert(handle->signum == 0); diff --git a/test/test-list.h b/test/test-list.h index 3a1e82a9..35656a76 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -354,6 +354,8 @@ TEST_DECLARE (spawn_fs_open) TEST_DECLARE (spawn_setuid_setgid) TEST_DECLARE (we_get_signal) TEST_DECLARE (we_get_signals) +TEST_DECLARE (we_get_signal_one_shot) +TEST_DECLARE (we_get_signals_mixed) TEST_DECLARE (signal_multiple_loops) TEST_DECLARE (closed_fd_events) #endif @@ -704,6 +706,8 @@ TASK_LIST_START TEST_ENTRY (spawn_setuid_setgid) TEST_ENTRY (we_get_signal) TEST_ENTRY (we_get_signals) + TEST_ENTRY (we_get_signal_one_shot) + TEST_ENTRY (we_get_signals_mixed) TEST_ENTRY (signal_multiple_loops) TEST_ENTRY (closed_fd_events) #endif diff --git a/test/test-signal.c b/test/test-signal.c index c0424c60..9a881510 100644 --- a/test/test-signal.c +++ b/test/test-signal.c @@ -70,17 +70,17 @@ struct timer_ctx { }; struct signal_ctx { - enum { CLOSE, STOP } stop_or_close; + enum { CLOSE, STOP, NOOP } stop_or_close; unsigned int ncalls; uv_signal_t handle; int signum; + int one_shot; }; static void signal_cb(uv_signal_t* handle, int signum) { struct signal_ctx* ctx = container_of(handle, struct signal_ctx, handle); ASSERT(signum == ctx->signum); - if (++ctx->ncalls == NSIGNALS) { if (ctx->stop_or_close == STOP) uv_signal_stop(handle); @@ -91,6 +91,14 @@ static void signal_cb(uv_signal_t* handle, int signum) { } } +static void signal_cb_one_shot(uv_signal_t* handle, int signum) { + struct signal_ctx* ctx = container_of(handle, struct signal_ctx, handle); + ASSERT(signum == ctx->signum); + ASSERT(++ctx->ncalls == 1); + if (ctx->stop_or_close == CLOSE) + uv_close((uv_handle_t*)handle, NULL); +} + static void timer_cb(uv_timer_t* handle) { struct timer_ctx* ctx = container_of(handle, struct timer_ctx, handle); @@ -102,15 +110,21 @@ static void timer_cb(uv_timer_t* handle) { } -static void start_watcher(uv_loop_t* loop, int signum, struct signal_ctx* ctx) { +static void start_watcher(uv_loop_t* loop, + int signum, + struct signal_ctx* ctx, + int one_shot) { ctx->ncalls = 0; ctx->signum = signum; ctx->stop_or_close = CLOSE; + ctx->one_shot = one_shot; ASSERT(0 == uv_signal_init(loop, &ctx->handle)); - ASSERT(0 == uv_signal_start(&ctx->handle, signal_cb, signum)); + if (one_shot) + ASSERT(0 == uv_signal_start_oneshot(&ctx->handle, signal_cb_one_shot, signum)); + else + ASSERT(0 == uv_signal_start(&ctx->handle, signal_cb, signum)); } - static void start_timer(uv_loop_t* loop, int signum, struct timer_ctx* ctx) { ctx->ncalls = 0; ctx->signum = signum; @@ -126,7 +140,7 @@ TEST_IMPL(we_get_signal) { loop = uv_default_loop(); start_timer(loop, SIGCHLD, &tc); - start_watcher(loop, SIGCHLD, &sc); + start_watcher(loop, SIGCHLD, &sc, 0); sc.stop_or_close = STOP; /* stop, don't close the signal handle */ ASSERT(0 == uv_run(loop, UV_RUN_DEFAULT)); ASSERT(tc.ncalls == NSIGNALS); @@ -158,10 +172,10 @@ TEST_IMPL(we_get_signals) { unsigned int i; loop = uv_default_loop(); - start_watcher(loop, SIGUSR1, sc + 0); - start_watcher(loop, SIGUSR1, sc + 1); - start_watcher(loop, SIGUSR2, sc + 2); - start_watcher(loop, SIGUSR2, sc + 3); + start_watcher(loop, SIGUSR1, sc + 0, 0); + start_watcher(loop, SIGUSR1, sc + 1, 0); + start_watcher(loop, SIGUSR2, sc + 2, 0); + start_watcher(loop, SIGUSR2, sc + 3, 0); start_timer(loop, SIGUSR1, tc + 0); start_timer(loop, SIGUSR2, tc + 1); ASSERT(0 == uv_run(loop, UV_RUN_DEFAULT)); @@ -176,4 +190,116 @@ TEST_IMPL(we_get_signals) { return 0; } +TEST_IMPL(we_get_signal_one_shot) { + struct signal_ctx sc; + struct timer_ctx tc; + uv_loop_t* loop; + + loop = uv_default_loop(); + start_timer(loop, SIGCHLD, &tc); + start_watcher(loop, SIGCHLD, &sc, 1); + sc.stop_or_close = NOOP; + ASSERT(0 == uv_run(loop, UV_RUN_DEFAULT)); + ASSERT(tc.ncalls == NSIGNALS); + ASSERT(sc.ncalls == 1); + + start_timer(loop, SIGCHLD, &tc); + ASSERT(0 == uv_run(loop, UV_RUN_DEFAULT)); + ASSERT(sc.ncalls == 1); + + sc.ncalls = 0; + sc.stop_or_close = CLOSE; /* now close it when it's done */ + uv_signal_start_oneshot(&sc.handle, signal_cb_one_shot, SIGCHLD); + start_timer(loop, SIGCHLD, &tc); + ASSERT(0 == uv_run(loop, UV_RUN_DEFAULT)); + ASSERT(tc.ncalls == NSIGNALS); + ASSERT(sc.ncalls == 1); + + MAKE_VALGRIND_HAPPY(); + return 0; +} + +TEST_IMPL(we_get_signals_mixed) { + struct signal_ctx sc[4]; + struct timer_ctx tc; + uv_loop_t* loop; + + loop = uv_default_loop(); + + /* 2 one-shot */ + start_timer(loop, SIGCHLD, &tc); + start_watcher(loop, SIGCHLD, sc + 0, 1); + start_watcher(loop, SIGCHLD, sc + 1, 1); + sc[0].stop_or_close = CLOSE; + sc[1].stop_or_close = CLOSE; + ASSERT(0 == uv_run(loop, UV_RUN_DEFAULT)); + ASSERT(tc.ncalls == NSIGNALS); + ASSERT(sc[0].ncalls == 1); + ASSERT(sc[1].ncalls == 1); + + /* 2 one-shot, 1 normal then remove normal */ + start_timer(loop, SIGCHLD, &tc); + start_watcher(loop, SIGCHLD, sc + 0, 1); + start_watcher(loop, SIGCHLD, sc + 1, 1); + sc[0].stop_or_close = CLOSE; + sc[1].stop_or_close = CLOSE; + start_watcher(loop, SIGCHLD, sc + 2, 0); + uv_close((uv_handle_t*)&(sc[2]).handle, NULL); + ASSERT(0 == uv_run(loop, UV_RUN_DEFAULT)); + ASSERT(tc.ncalls == NSIGNALS); + ASSERT(sc[0].ncalls == 1); + ASSERT(sc[1].ncalls == 1); + ASSERT(sc[2].ncalls == 0); + + /* 2 normal, 1 one-shot then remove one-shot */ + start_timer(loop, SIGCHLD, &tc); + start_watcher(loop, SIGCHLD, sc + 0, 0); + start_watcher(loop, SIGCHLD, sc + 1, 0); + sc[0].stop_or_close = CLOSE; + sc[1].stop_or_close = CLOSE; + start_watcher(loop, SIGCHLD, sc + 2, 1); + uv_close((uv_handle_t*)&(sc[2]).handle, NULL); + ASSERT(0 == uv_run(loop, UV_RUN_DEFAULT)); + ASSERT(tc.ncalls == NSIGNALS); + ASSERT(sc[0].ncalls == NSIGNALS); + ASSERT(sc[1].ncalls == NSIGNALS); + ASSERT(sc[2].ncalls == 0); + + /* 2 normal, 2 one-shot then remove 2 normal */ + start_timer(loop, SIGCHLD, &tc); + start_watcher(loop, SIGCHLD, sc + 0, 0); + start_watcher(loop, SIGCHLD, sc + 1, 0); + start_watcher(loop, SIGCHLD, sc + 2, 1); + start_watcher(loop, SIGCHLD, sc + 3, 1); + sc[2].stop_or_close = CLOSE; + sc[3].stop_or_close = CLOSE; + uv_close((uv_handle_t*)&(sc[0]).handle, NULL); + uv_close((uv_handle_t*)&(sc[1]).handle, NULL); + ASSERT(0 == uv_run(loop, UV_RUN_DEFAULT)); + ASSERT(tc.ncalls == NSIGNALS); + ASSERT(sc[0].ncalls == 0); + ASSERT(sc[1].ncalls == 0); + ASSERT(sc[2].ncalls == 1); + ASSERT(sc[2].ncalls == 1); + + /* 1 normal, 1 one-shot, 2 normal then remove 1st normal, 2nd normal */ + start_timer(loop, SIGCHLD, &tc); + start_watcher(loop, SIGCHLD, sc + 0, 0); + start_watcher(loop, SIGCHLD, sc + 1, 1); + start_watcher(loop, SIGCHLD, sc + 2, 0); + start_watcher(loop, SIGCHLD, sc + 3, 0); + sc[3].stop_or_close = CLOSE; + uv_close((uv_handle_t*)&(sc[0]).handle, NULL); + uv_close((uv_handle_t*)&(sc[2]).handle, NULL); + ASSERT(0 == uv_run(loop, UV_RUN_DEFAULT)); + ASSERT(tc.ncalls == NSIGNALS); + ASSERT(sc[0].ncalls == 0); + ASSERT(sc[1].ncalls == 1); + ASSERT(sc[2].ncalls == 0); + ASSERT(sc[3].ncalls == NSIGNALS); + + MAKE_VALGRIND_HAPPY(); + return 0; +} + #endif /* _WIN32 */