Skip to content

Commit

Permalink
win, pipe: avoid synchronous pipe deadlocks
Browse files Browse the repository at this point in the history
Add a thread that will interrupt uv_pipe_zero_readdile_thread_proc every half
second. This allows other processes to access the pipe without deadlocking

Ref: nodejs/node#10836
  • Loading branch information
bzoz committed Apr 26, 2017
1 parent 710989b commit 0fa84df
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 0 deletions.
29 changes: 29 additions & 0 deletions src/win/pipe.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ struct uv__ipc_queue_item_s {
/* A zero-size buffer for use by uv_pipe_read */
static char uv_zero_[] = "";

/* To prevent deadlocks we will interrupt synchronous pipe read every 500 ms */
#define PIPE_ZERO_READ_INTERRUPT_INTERVAL 500

/* Null uv_buf_t */
static const uv_buf_t uv_null_buf_ = { 0, NULL };

Expand Down Expand Up @@ -942,13 +945,33 @@ int uv_pipe_listen(uv_pipe_t* handle, int backlog, uv_connection_cb cb) {
}


static DWORD WINAPI uv__pipe_zero_read_interrupter(void* param) {
HANDLE thread;
int r;
thread = (HANDLE) param;
for (;;) {
r = WaitForSingleObject(thread, PIPE_ZERO_READ_INTERRUPT_INTERVAL);
if (r == WAIT_TIMEOUT) {
/* ReadFile thread is active - interrupt ReadFile to give other */
/* processes a chance to act upon pipe */
pCancelSynchronousIo(thread);
} else {
/* ReadFile thread has terminated - cleanup the handle and exit */
CloseHandle(thread);
return 0;
}
}
}


static DWORD WINAPI uv_pipe_zero_readfile_thread_proc(void* parameter) {
int result;
DWORD bytes;
uv_read_t* req = (uv_read_t*) parameter;
uv_pipe_t* handle = (uv_pipe_t*) req->data;
uv_loop_t* loop = handle->loop;
HANDLE hThread = NULL;
HANDLE hThreadForInterrupt = NULL;
DWORD err;
uv_mutex_t *m = &handle->pipe.conn.readfile_mutex;

Expand All @@ -965,6 +988,12 @@ static DWORD WINAPI uv_pipe_zero_readfile_thread_proc(void* parameter) {
} else {
hThread = NULL;
}
DuplicateHandle(GetCurrentProcess(), GetCurrentThread(),
GetCurrentProcess(), &hThreadForInterrupt,
0, TRUE, DUPLICATE_SAME_ACCESS);
QueueUserWorkItem(uv__pipe_zero_read_interrupter,
hThreadForInterrupt,
WT_EXECUTEINLONGTHREAD);
uv_mutex_unlock(m);
}
restart_readfile:
Expand Down
6 changes: 6 additions & 0 deletions test/test-list.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ TEST_DECLARE (pipe_ref4)
TEST_DECLARE (pipe_close_stdout_read_stdin)
#endif
TEST_DECLARE (pipe_set_non_blocking)
#ifdef _WIN32
TEST_DECLARE (pipe_open_read_pipe)
#endif
TEST_DECLARE (process_ref)
TEST_DECLARE (has_ref)
TEST_DECLARE (active)
Expand Down Expand Up @@ -422,6 +425,9 @@ TASK_LIST_START
TEST_ENTRY (pipe_close_stdout_read_stdin)
#endif
TEST_ENTRY (pipe_set_non_blocking)
#ifdef _WIN32
TEST_ENTRY (pipe_open_read_pipe)
#endif
TEST_ENTRY (tty)
#ifdef _WIN32
TEST_ENTRY (tty_raw)
Expand Down
61 changes: 61 additions & 0 deletions test/test-pipe-open-read-pipe.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#include "uv.h"
#include "task.h"

#ifndef _WIN32
TEST_IMPL(pipe_open_read_pipe) {
RETURN_SKIP("Test only for Windows.");
}
#else

#define NAME_BUF_SIZE 32768
#define CHILD_WAIT_TIMEOUT 1000

void alloc_cb(uv_handle_t* handle, size_t suggested_size, uv_buf_t* buf) {
buf->base = malloc(suggested_size);
buf->len = suggested_size;
}

void read_cb(uv_stream_t* stream, ssize_t nread, const uv_buf_t* buf) {
}

void pipe_read_thread_proc(void* arg) {
uv_pipe_t* pipe = arg;
uv_read_start(pipe, alloc_cb, read_cb);
uv_run(uv_default_loop(), UV_RUN_DEFAULT);
FATAL("loop should not exit");
}

TEST_IMPL(pipe_open_read_pipe) {
int r, pipe_fd;
uv_thread_t pipe_read_thread;
uv_pipe_t uv_pipe, uv_reopen_pipe;
HANDLE stdin_read_pipe, stdin_write_pipe;
SECURITY_ATTRIBUTES sa_attr;

sa_attr.nLength = sizeof(SECURITY_ATTRIBUTES);
sa_attr.bInheritHandle = TRUE;
sa_attr.lpSecurityDescriptor = NULL;
r = CreatePipe(&stdin_read_pipe, &stdin_write_pipe, &sa_attr, 0);
ASSERT(r != 0);

r = uv_pipe_init(uv_default_loop(), &uv_pipe, 0);
ASSERT(r == 0);
pipe_fd = _open_osfhandle(stdin_read_pipe, 0);
r = uv_pipe_open(&uv_pipe, pipe_fd);
ASSERT(r == 0);

r = uv_thread_create(&pipe_read_thread, pipe_read_thread_proc, &uv_pipe);
ASSERT(r == 0);

/* Give uv_run some time to start */
uv_sleep(250);
/* Try to access the pipe again, in different loop */
uv_loop_t test_loop;
r = uv_loop_init(&test_loop);
ASSERT(r == 0);
r = uv_pipe_init(&test_loop, &uv_reopen_pipe, 0);
ASSERT(r == 0);
r = uv_pipe_open(&uv_reopen_pipe, pipe_fd);
return TEST_OK;
}
#endif
1 change: 1 addition & 0 deletions uv.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@
'test/test-pipe-server-close.c',
'test/test-pipe-close-stdout-read-stdin.c',
'test/test-pipe-set-non-blocking.c',
'test/test-pipe-open-read-pipe.c',
'test/test-platform-output.c',
'test/test-poll.c',
'test/test-poll-close.c',
Expand Down

0 comments on commit 0fa84df

Please sign in to comment.