unix,win: fix UV_RUN_ONCE + uv_idle_stop loop hang (#3590)

Wrong accounting of idle handles in uv_run() made it sleep when there
was nothing left to do. Do a non-blocking poll for I/O instead.

5 files changed, 44 insertions, 17 deletions

diff --git a/src/unix/core.c b/src/unix/core.c
index edd457a1..ae160f0c 100644
--- a/src/unix/core.c
+++ b/src/unix/core.c
@@ -94,7 +94,7 @@ extern char** environ;
 # include <sanitizer/linux_syscall_hooks.h>
 #endif
 
-static int uv__run_pending(uv_loop_t* loop);
+static void uv__run_pending(uv_loop_t* loop);
 
 /* Verify that uv_buf_t is ABI-compatible with struct iovec. */
 STATIC_ASSERT(sizeof(uv_buf_t) == sizeof(struct iovec));
@@ -372,7 +372,7 @@ int uv_loop_alive(const uv_loop_t* loop) {
 int uv_run(uv_loop_t* loop, uv_run_mode mode) {
   int timeout;
   int r;
-  int ran_pending;
+  int can_sleep;
 
   r = uv__loop_alive(loop);
   if (!r)
@@ -381,12 +381,16 @@ int uv_run(uv_loop_t* loop, uv_run_mode mode) {
   while (r != 0 && loop->stop_flag == 0) {
     uv__update_time(loop);
     uv__run_timers(loop);
-    ran_pending = uv__run_pending(loop);
+
+    can_sleep =
+        QUEUE_EMPTY(&loop->pending_queue) && QUEUE_EMPTY(&loop->idle_handles);
+
+    uv__run_pending(loop);
     uv__run_idle(loop);
     uv__run_prepare(loop);
 
     timeout = 0;
-    if ((mode == UV_RUN_ONCE && !ran_pending) || mode == UV_RUN_DEFAULT)
+    if ((mode == UV_RUN_ONCE && can_sleep) || mode == UV_RUN_DEFAULT)
       timeout = uv__backend_timeout(loop);
 
     uv__io_poll(loop, timeout);
@@ -780,14 +784,11 @@ int uv_fileno(const uv_handle_t* handle, uv_os_fd_t* fd) {
 }
 
 
-static int uv__run_pending(uv_loop_t* loop) {
+static void uv__run_pending(uv_loop_t* loop) {
   QUEUE* q;
   QUEUE pq;
   uv__io_t* w;
 
-  if (QUEUE_EMPTY(&loop->pending_queue))
-    return 0;
-
   QUEUE_MOVE(&loop->pending_queue, &pq);
 
   while (!QUEUE_EMPTY(&pq)) {
@@ -797,8 +798,6 @@ static int uv__run_pending(uv_loop_t* loop) {
     w = QUEUE_DATA(q, uv__io_t, pending_queue);
     w->cb(loop, w, POLLOUT);
   }
-
-  return 1;
 }
 
 
diff --git a/src/win/core.c b/src/win/core.c
index c08dedc4..664ffac9 100644
--- a/src/win/core.c
+++ b/src/win/core.c
@@ -592,7 +592,7 @@ static void uv__poll(uv_loop_t* loop, DWORD timeout) {
 int uv_run(uv_loop_t *loop, uv_run_mode mode) {
   DWORD timeout;
   int r;
-  int ran_pending;
+  int can_sleep;
 
   r = uv__loop_alive(loop);
   if (!r)
@@ -602,12 +602,14 @@ int uv_run(uv_loop_t *loop, uv_run_mode mode) {
     uv_update_time(loop);
     uv__run_timers(loop);
 
-    ran_pending = uv__process_reqs(loop);
+    can_sleep = loop->pending_reqs_tail == NULL && loop->idle_handles == NULL;
+
+    uv__process_reqs(loop);
     uv__idle_invoke(loop);
     uv__prepare_invoke(loop);
 
     timeout = 0;
-    if ((mode == UV_RUN_ONCE && !ran_pending) || mode == UV_RUN_DEFAULT)
+    if ((mode == UV_RUN_ONCE && can_sleep) || mode == UV_RUN_DEFAULT)
       timeout = uv_backend_timeout(loop);
 
     if (pGetQueuedCompletionStatusEx)
diff --git a/src/win/req-inl.h b/src/win/req-inl.h
index b7a34564..48760a06 100644
--- a/src/win/req-inl.h
+++ b/src/win/req-inl.h
@@ -138,13 +138,13 @@ INLINE static void uv__insert_pending_req(uv_loop_t* loop, uv_req_t* req) {
   } while (0)
 
 
-INLINE static int uv__process_reqs(uv_loop_t* loop) {
+INLINE static void uv__process_reqs(uv_loop_t* loop) {
   uv_req_t* req;
   uv_req_t* first;
   uv_req_t* next;
 
   if (loop->pending_reqs_tail == NULL)
-    return 0;
+    return;
 
   first = loop->pending_reqs_tail->next_req;
   next = first;
@@ -214,8 +214,6 @@ INLINE static int uv__process_reqs(uv_loop_t* loop) {
         assert(0);
     }
   }
-
-  return 1;
 }
 
 #endif /* UV_WIN_REQ_INL_H_ */
diff --git a/test/test-idle.c b/test/test-idle.c
index f49d1964..427cf545 100644
--- a/test/test-idle.c
+++ b/test/test-idle.c
@@ -97,3 +97,29 @@ TEST_IMPL(idle_starvation) {
   MAKE_VALGRIND_HAPPY();
   return 0;
 }
+
+
+static void idle_stop(uv_idle_t* handle) {
+  uv_idle_stop(handle);
+}
+
+
+TEST_IMPL(idle_check) {
+  ASSERT_EQ(0, uv_idle_init(uv_default_loop(), &idle_handle));
+  ASSERT_EQ(0, uv_idle_start(&idle_handle, idle_stop));
+
+  ASSERT_EQ(0, uv_check_init(uv_default_loop(), &check_handle));
+  ASSERT_EQ(0, uv_check_start(&check_handle, check_cb));
+
+  ASSERT_EQ(1, uv_run(uv_default_loop(), UV_RUN_ONCE));
+  ASSERT_EQ(1, check_cb_called);
+
+  ASSERT_EQ(0, close_cb_called);
+  uv_close((uv_handle_t*) &idle_handle, close_cb);
+  uv_close((uv_handle_t*) &check_handle, close_cb);
+  ASSERT_EQ(0, uv_run(uv_default_loop(), UV_RUN_ONCE));
+  ASSERT_EQ(2, close_cb_called);
+
+  MAKE_VALGRIND_HAPPY();
+  return 0;
+}
diff --git a/test/test-list.h b/test/test-list.h
index 44751658..26c14b06 100644
--- a/test/test-list.h
+++ b/test/test-list.h
@@ -225,6 +225,7 @@ TEST_DECLARE   (timer_is_closing)
 TEST_DECLARE   (timer_null_callback)
 TEST_DECLARE   (timer_early_check)
 TEST_DECLARE   (idle_starvation)
+TEST_DECLARE   (idle_check)
 TEST_DECLARE   (loop_handles)
 TEST_DECLARE   (get_loadavg)
 TEST_DECLARE   (walk_handles)
@@ -819,6 +820,7 @@ TASK_LIST_START
   TEST_ENTRY  (timer_early_check)
 
   TEST_ENTRY  (idle_starvation)
+  TEST_ENTRY  (idle_check)
 
   TEST_ENTRY  (ref)
   TEST_ENTRY  (idle_ref)