mirror of
https://github.com/libuv/libuv
synced 2025-03-28 21:13:16 +00:00
win,signal: fix potential deadlock
Trying to remove the controller from the controller handle itself leads to deadlock. Fix it by always having the global signal handler engaged. Ref: https://github.com/nodejs/node/issues/10165 PR-URL: https://github.com/libuv/libuv/pull/1168 Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
This commit is contained in:
parent
c8ab24bb92
commit
c66f265b5d
118
src/win/signal.c
118
src/win/signal.c
@ -30,12 +30,14 @@
|
||||
RB_HEAD(uv_signal_tree_s, uv_signal_s);
|
||||
|
||||
static struct uv_signal_tree_s uv__signal_tree = RB_INITIALIZER(uv__signal_tree);
|
||||
static ssize_t volatile uv__signal_control_handler_refs = 0;
|
||||
static CRITICAL_SECTION uv__signal_lock;
|
||||
|
||||
static BOOL WINAPI uv__signal_control_handler(DWORD type);
|
||||
|
||||
void uv_signals_init() {
|
||||
InitializeCriticalSection(&uv__signal_lock);
|
||||
if (!SetConsoleCtrlHandler(uv__signal_control_handler, TRUE))
|
||||
abort();
|
||||
}
|
||||
|
||||
|
||||
@ -125,102 +127,6 @@ static BOOL WINAPI uv__signal_control_handler(DWORD type) {
|
||||
}
|
||||
|
||||
|
||||
static int uv__signal_register_control_handler() {
|
||||
/* When this function is called, the uv__signal_lock must be held. */
|
||||
|
||||
/* If the console control handler has already been hooked, just add a */
|
||||
/* reference. */
|
||||
if (uv__signal_control_handler_refs > 0) {
|
||||
uv__signal_control_handler_refs++;
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (!SetConsoleCtrlHandler(uv__signal_control_handler, TRUE))
|
||||
return GetLastError();
|
||||
|
||||
uv__signal_control_handler_refs++;
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
static void uv__signal_unregister_control_handler() {
|
||||
/* When this function is called, the uv__signal_lock must be held. */
|
||||
BOOL r;
|
||||
|
||||
/* Don't unregister if the number of console control handlers exceeds one. */
|
||||
/* Just remove a reference in that case. */
|
||||
if (uv__signal_control_handler_refs > 1) {
|
||||
uv__signal_control_handler_refs--;
|
||||
return;
|
||||
}
|
||||
|
||||
assert(uv__signal_control_handler_refs == 1);
|
||||
|
||||
r = SetConsoleCtrlHandler(uv__signal_control_handler, FALSE);
|
||||
/* This should never fail; if it does it is probably a bug in libuv. */
|
||||
assert(r);
|
||||
|
||||
uv__signal_control_handler_refs--;
|
||||
}
|
||||
|
||||
|
||||
static int uv__signal_register(int signum) {
|
||||
switch (signum) {
|
||||
case SIGINT:
|
||||
case SIGBREAK:
|
||||
case SIGHUP:
|
||||
return uv__signal_register_control_handler();
|
||||
|
||||
case SIGWINCH:
|
||||
/* SIGWINCH is generated in tty.c. No need to register anything. */
|
||||
return 0;
|
||||
|
||||
case SIGILL:
|
||||
case SIGABRT_COMPAT:
|
||||
case SIGFPE:
|
||||
case SIGSEGV:
|
||||
case SIGTERM:
|
||||
case SIGABRT:
|
||||
/* Signal is never raised. */
|
||||
return 0;
|
||||
|
||||
default:
|
||||
/* Invalid signal. */
|
||||
return ERROR_INVALID_PARAMETER;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
static void uv__signal_unregister(int signum) {
|
||||
switch (signum) {
|
||||
case SIGINT:
|
||||
case SIGBREAK:
|
||||
case SIGHUP:
|
||||
uv__signal_unregister_control_handler();
|
||||
return;
|
||||
|
||||
case SIGWINCH:
|
||||
/* SIGWINCH is generated in tty.c. No need to unregister anything. */
|
||||
return;
|
||||
|
||||
case SIGILL:
|
||||
case SIGABRT_COMPAT:
|
||||
case SIGFPE:
|
||||
case SIGSEGV:
|
||||
case SIGTERM:
|
||||
case SIGABRT:
|
||||
/* Nothing is registered for this signal. */
|
||||
return;
|
||||
|
||||
default:
|
||||
/* Libuv bug. */
|
||||
assert(0 && "Invalid signum");
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
int uv_signal_init(uv_loop_t* loop, uv_signal_t* handle) {
|
||||
uv_req_t* req;
|
||||
|
||||
@ -247,8 +153,6 @@ int uv_signal_stop(uv_signal_t* handle) {
|
||||
|
||||
EnterCriticalSection(&uv__signal_lock);
|
||||
|
||||
uv__signal_unregister(handle->signum);
|
||||
|
||||
removed_handle = RB_REMOVE(uv_signal_tree_s, &uv__signal_tree, handle);
|
||||
assert(removed_handle == handle);
|
||||
|
||||
@ -262,14 +166,9 @@ int uv_signal_stop(uv_signal_t* handle) {
|
||||
|
||||
|
||||
int uv_signal_start(uv_signal_t* handle, uv_signal_cb signal_cb, int signum) {
|
||||
int err;
|
||||
|
||||
/* If the user supplies signum == 0, then return an error already. If the */
|
||||
/* signum is otherwise invalid then uv__signal_register will find out */
|
||||
/* eventually. */
|
||||
if (signum == 0) {
|
||||
/* Test for invalid signal values. */
|
||||
if (signum != SIGWINCH && (signum <= 0 || signum >= NSIG))
|
||||
return UV_EINVAL;
|
||||
}
|
||||
|
||||
/* Short circuit: if the signal watcher is already watching {signum} don't */
|
||||
/* go through the process of deregistering and registering the handler. */
|
||||
@ -289,13 +188,6 @@ int uv_signal_start(uv_signal_t* handle, uv_signal_cb signal_cb, int signum) {
|
||||
|
||||
EnterCriticalSection(&uv__signal_lock);
|
||||
|
||||
err = uv__signal_register(signum);
|
||||
if (err) {
|
||||
/* Uh-oh, didn't work. */
|
||||
LeaveCriticalSection(&uv__signal_lock);
|
||||
return uv_translate_sys_error(err);
|
||||
}
|
||||
|
||||
handle->signum = signum;
|
||||
RB_INSERT(uv_signal_tree_s, &uv__signal_tree, handle);
|
||||
|
||||
|
@ -346,6 +346,7 @@ TEST_DECLARE (listen_no_simultaneous_accepts)
|
||||
TEST_DECLARE (fs_stat_root)
|
||||
TEST_DECLARE (spawn_with_an_odd_path)
|
||||
TEST_DECLARE (ipc_listen_after_bind_twice)
|
||||
TEST_DECLARE (win32_signum_number)
|
||||
#else
|
||||
TEST_DECLARE (emfile)
|
||||
TEST_DECLARE (close_fd)
|
||||
@ -695,6 +696,7 @@ TASK_LIST_START
|
||||
TEST_ENTRY (fs_stat_root)
|
||||
TEST_ENTRY (spawn_with_an_odd_path)
|
||||
TEST_ENTRY (ipc_listen_after_bind_twice)
|
||||
TEST_ENTRY (win32_signum_number)
|
||||
#else
|
||||
TEST_ENTRY (emfile)
|
||||
TEST_ENTRY (close_fd)
|
||||
|
@ -19,13 +19,40 @@
|
||||
* IN THE SOFTWARE.
|
||||
*/
|
||||
|
||||
|
||||
/* This test does not pretend to be cross-platform. */
|
||||
#ifndef _WIN32
|
||||
|
||||
#include "uv.h"
|
||||
#include "task.h"
|
||||
|
||||
/* For Windows we test only signum handling */
|
||||
#ifdef _WIN32
|
||||
static void signum_test_cb(uv_signal_t* handle, int signum) {
|
||||
FATAL("signum_test_cb should not be called");
|
||||
}
|
||||
|
||||
TEST_IMPL(win32_signum_number) {
|
||||
uv_signal_t signal;
|
||||
uv_loop_t* loop;
|
||||
|
||||
loop = uv_default_loop();
|
||||
uv_signal_init(loop, &signal);
|
||||
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, 0) == UV_EINVAL);
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, SIGINT) == 0);
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, SIGBREAK) == 0);
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, SIGHUP) == 0);
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, SIGWINCH) == 0);
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, SIGILL) == 0);
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, SIGABRT_COMPAT) == 0);
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, SIGFPE) == 0);
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, SIGSEGV) == 0);
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, SIGTERM) == 0);
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, SIGABRT) == 0);
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, -1) == UV_EINVAL);
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, NSIG) == UV_EINVAL);
|
||||
ASSERT(uv_signal_start(&signal, signum_test_cb, 1024) == UV_EINVAL);
|
||||
MAKE_VALGRIND_HAPPY();
|
||||
return 0;
|
||||
}
|
||||
#else
|
||||
#include <errno.h>
|
||||
#include <signal.h>
|
||||
#include <stdarg.h>
|
||||
|
Loading…
x
Reference in New Issue
Block a user