diff --git a/src/fs-poll.c b/src/fs-poll.c index 6c82dfc1..40cb147e 100644 --- a/src/fs-poll.c +++ b/src/fs-poll.c @@ -22,12 +22,20 @@ #include "uv.h" #include "uv-common.h" +#ifdef _WIN32 +#include "win/internal.h" +#include "win/handle-inl.h" +#define uv__make_close_pending(h) uv_want_endgame((h)->loop, (h)) +#else +#include "unix/internal.h" +#endif + #include #include #include struct poll_ctx { - uv_fs_poll_t* parent_handle; /* NULL if parent has been stopped or closed */ + uv_fs_poll_t* parent_handle; int busy_polling; unsigned int interval; uint64_t start_time; @@ -36,6 +44,7 @@ struct poll_ctx { uv_timer_t timer_handle; uv_fs_t fs_req; /* TODO(bnoordhuis) mark fs_req internal */ uv_stat_t statbuf; + struct poll_ctx* previous; /* context from previous start()..stop() period */ char path[1]; /* variable length */ }; @@ -49,6 +58,7 @@ static uv_stat_t zero_statbuf; int uv_fs_poll_init(uv_loop_t* loop, uv_fs_poll_t* handle) { uv__handle_init(loop, (uv_handle_t*)handle, UV_FS_POLL); + handle->poll_ctx = NULL; return 0; } @@ -62,7 +72,7 @@ int uv_fs_poll_start(uv_fs_poll_t* handle, size_t len; int err; - if (uv__is_active(handle)) + if (uv_is_active((uv_handle_t*)handle)) return 0; loop = handle->loop; @@ -90,6 +100,8 @@ int uv_fs_poll_start(uv_fs_poll_t* handle, if (err < 0) goto error; + if (handle->poll_ctx != NULL) + ctx->previous = handle->poll_ctx; handle->poll_ctx = ctx; uv__handle_start(handle); @@ -104,19 +116,17 @@ error: int uv_fs_poll_stop(uv_fs_poll_t* handle) { struct poll_ctx* ctx; - if (!uv__is_active(handle)) + if (!uv_is_active((uv_handle_t*)handle)) return 0; ctx = handle->poll_ctx; assert(ctx != NULL); - assert(ctx->parent_handle != NULL); - ctx->parent_handle = NULL; - handle->poll_ctx = NULL; + assert(ctx->parent_handle == handle); /* Close the timer if it's active. If it's inactive, there's a stat request * in progress and poll_cb will take care of the cleanup. */ - if (uv__is_active(&ctx->timer_handle)) + if (uv_is_active((uv_handle_t*)&ctx->timer_handle)) uv_close((uv_handle_t*)&ctx->timer_handle, timer_close_cb); uv__handle_stop(handle); @@ -129,7 +139,7 @@ int uv_fs_poll_getpath(uv_fs_poll_t* handle, char* buffer, size_t* size) { struct poll_ctx* ctx; size_t required_len; - if (!uv__is_active(handle)) { + if (!uv_is_active((uv_handle_t*)handle)) { *size = 0; return UV_EINVAL; } @@ -153,6 +163,9 @@ int uv_fs_poll_getpath(uv_fs_poll_t* handle, char* buffer, size_t* size) { void uv__fs_poll_close(uv_fs_poll_t* handle) { uv_fs_poll_stop(handle); + + if (handle->poll_ctx == NULL) + uv__make_close_pending((uv_handle_t*)handle); } @@ -173,14 +186,13 @@ static void poll_cb(uv_fs_t* req) { uv_stat_t* statbuf; struct poll_ctx* ctx; uint64_t interval; + uv_fs_poll_t* handle; ctx = container_of(req, struct poll_ctx, fs_req); + handle = ctx->parent_handle; - if (ctx->parent_handle == NULL) { /* handle has been stopped or closed */ - uv_close((uv_handle_t*)&ctx->timer_handle, timer_close_cb); - uv_fs_req_cleanup(req); - return; - } + if (!uv_is_active((uv_handle_t*)handle) || uv__is_closing(handle)) + goto out; if (req->result != 0) { if (ctx->busy_polling != req->result) { @@ -205,7 +217,7 @@ static void poll_cb(uv_fs_t* req) { out: uv_fs_req_cleanup(req); - if (ctx->parent_handle == NULL) { /* handle has been stopped by callback */ + if (!uv_is_active((uv_handle_t*)handle) || uv__is_closing(handle)) { uv_close((uv_handle_t*)&ctx->timer_handle, timer_close_cb); return; } @@ -219,8 +231,27 @@ out: } -static void timer_close_cb(uv_handle_t* handle) { - uv__free(container_of(handle, struct poll_ctx, timer_handle)); +static void timer_close_cb(uv_handle_t* timer) { + struct poll_ctx* ctx; + struct poll_ctx* it; + struct poll_ctx* last; + uv_fs_poll_t* handle; + + ctx = container_of(timer, struct poll_ctx, timer_handle); + handle = ctx->parent_handle; + if (ctx == handle->poll_ctx) { + handle->poll_ctx = ctx->previous; + if (handle->poll_ctx == NULL) + uv__make_close_pending((uv_handle_t*)handle); + } else { + for (last = handle->poll_ctx, it = last->previous; + it != ctx; + last = it, it = it->previous) { + assert(last->previous != NULL); + } + last->previous = ctx->previous; + } + uv__free(ctx); } diff --git a/src/unix/core.c b/src/unix/core.c index 7e42337b..475b081e 100644 --- a/src/unix/core.c +++ b/src/unix/core.c @@ -161,7 +161,9 @@ void uv_close(uv_handle_t* handle, uv_close_cb close_cb) { case UV_FS_POLL: uv__fs_poll_close((uv_fs_poll_t*)handle); - break; + /* Poll handles use file system requests, and one of them may still be + * running. The poll code will call uv__make_close_pending() for us. */ + return; case UV_SIGNAL: uv__signal_close((uv_signal_t*) handle); diff --git a/src/win/handle.c b/src/win/handle.c index 9d76c3f5..61e4df61 100644 --- a/src/win/handle.c +++ b/src/win/handle.c @@ -139,7 +139,6 @@ void uv_close(uv_handle_t* handle, uv_close_cb cb) { case UV_FS_POLL: uv__fs_poll_close((uv_fs_poll_t*) handle); uv__handle_closing(handle); - uv_want_endgame(loop, handle); return; default: diff --git a/test/test-fs-poll.c b/test/test-fs-poll.c index 737d50df..e19a6878 100644 --- a/test/test-fs-poll.c +++ b/test/test-fs-poll.c @@ -185,3 +185,77 @@ TEST_IMPL(fs_poll_getpath) { MAKE_VALGRIND_HAPPY(); return 0; } + + +TEST_IMPL(fs_poll_close_request) { + uv_loop_t loop; + uv_fs_poll_t poll_handle; + + remove(FIXTURE); + + ASSERT(0 == uv_loop_init(&loop)); + + ASSERT(0 == uv_fs_poll_init(&loop, &poll_handle)); + ASSERT(0 == uv_fs_poll_start(&poll_handle, poll_cb_fail, FIXTURE, 100)); + uv_close((uv_handle_t*) &poll_handle, close_cb); + while (close_cb_called == 0) + uv_run(&loop, UV_RUN_ONCE); + ASSERT(close_cb_called == 1); + + ASSERT(0 == uv_loop_close(&loop)); + + MAKE_VALGRIND_HAPPY(); + return 0; +} + +TEST_IMPL(fs_poll_close_request_multi_start_stop) { + uv_loop_t loop; + uv_fs_poll_t poll_handle; + int i; + + remove(FIXTURE); + + ASSERT(0 == uv_loop_init(&loop)); + + ASSERT(0 == uv_fs_poll_init(&loop, &poll_handle)); + + for (i = 0; i < 10; ++i) { + ASSERT(0 == uv_fs_poll_start(&poll_handle, poll_cb_fail, FIXTURE, 100)); + ASSERT(0 == uv_fs_poll_stop(&poll_handle)); + } + uv_close((uv_handle_t*) &poll_handle, close_cb); + while (close_cb_called == 0) + uv_run(&loop, UV_RUN_ONCE); + ASSERT(close_cb_called == 1); + + ASSERT(0 == uv_loop_close(&loop)); + + MAKE_VALGRIND_HAPPY(); + return 0; +} + +TEST_IMPL(fs_poll_close_request_multi_stop_start) { + uv_loop_t loop; + uv_fs_poll_t poll_handle; + int i; + + remove(FIXTURE); + + ASSERT(0 == uv_loop_init(&loop)); + + ASSERT(0 == uv_fs_poll_init(&loop, &poll_handle)); + + for (i = 0; i < 10; ++i) { + ASSERT(0 == uv_fs_poll_stop(&poll_handle)); + ASSERT(0 == uv_fs_poll_start(&poll_handle, poll_cb_fail, FIXTURE, 100)); + } + uv_close((uv_handle_t*) &poll_handle, close_cb); + while (close_cb_called == 0) + uv_run(&loop, UV_RUN_ONCE); + ASSERT(close_cb_called == 1); + + ASSERT(0 == uv_loop_close(&loop)); + + MAKE_VALGRIND_HAPPY(); + return 0; +} diff --git a/test/test-list.h b/test/test-list.h index d81416fa..6d2729dc 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -280,6 +280,9 @@ TEST_DECLARE (spawn_quoted_path) TEST_DECLARE (spawn_tcp_server) TEST_DECLARE (fs_poll) TEST_DECLARE (fs_poll_getpath) +TEST_DECLARE (fs_poll_close_request) +TEST_DECLARE (fs_poll_close_request_multi_start_stop) +TEST_DECLARE (fs_poll_close_request_multi_stop_start) TEST_DECLARE (kill) TEST_DECLARE (kill_invalid_signum) TEST_DECLARE (fs_file_noent) @@ -817,6 +820,9 @@ TASK_LIST_START TEST_ENTRY (spawn_tcp_server) TEST_ENTRY (fs_poll) TEST_ENTRY (fs_poll_getpath) + TEST_ENTRY (fs_poll_close_request) + TEST_ENTRY (fs_poll_close_request_multi_start_stop) + TEST_ENTRY (fs_poll_close_request_multi_stop_start) TEST_ENTRY (kill) TEST_ENTRY (kill_invalid_signum)