From 37042a5bca8b392f71d7da6e01457ece5a148e99 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Tue, 26 Mar 2019 02:06:32 +0100
Subject: [PATCH] unix: fix race condition in uv_async_send()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After inspection of the code, it seems like there is a race window
between the cmpxchgi() and uv__async_send() calls.

If the event loop thread is already busy looping over the async handles,
it can invoke the callback - which in turn can close the handle - before
the other thread reaches the uv__async_send() call. That's bad because
it accesses the handle that at that point might not be valid anymore.

Fix that by introducing an ad hoc spinlock that blocks the event loop
thread until the sending thread is done. It's not pretty or elegant
but it fixes the immediate bug and appears to have no measurable
impact on the async handle benchmarks.

Fixes: https://github.com/libuv/libuv/issues/2226
PR-URL: https://github.com/libuv/libuv/pull/2231
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
---
 src/unix/async.c | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/src/unix/async.c b/src/unix/async.c
index 0b450ae0..a5c47bca 100644
--- a/src/unix/async.c
+++ b/src/unix/async.c
@@ -61,14 +61,43 @@ int uv_async_send(uv_async_t* handle) {
   if (ACCESS_ONCE(int, handle->pending) != 0)
     return 0;
 
-  if (cmpxchgi(&handle->pending, 0, 1) == 0)
-    uv__async_send(handle->loop);
+  /* Tell the other thread we're busy with the handle. */
+  if (cmpxchgi(&handle->pending, 0, 1) != 0)
+    return 0;
+
+  /* Wake up the other thread's event loop. */
+  uv__async_send(handle->loop);
+
+  /* Tell the other thread we're done. */
+  if (cmpxchgi(&handle->pending, 1, 2) != 1)
+    abort();
 
   return 0;
 }
 
 
+/* Only call this from the event loop thread. */
+static int uv__async_spin(uv_async_t* handle) {
+  int rc;
+
+  for (;;) {
+    /* rc=0 -- handle is not pending.
+     * rc=1 -- handle is pending, other thread is still working with it.
+     * rc=2 -- handle is pending, other thread is done.
+     */
+    rc = cmpxchgi(&handle->pending, 2, 0);
+
+    if (rc != 1)
+      return rc;
+
+    /* Other thread is busy with this handle, spin until it's done. */
+    cpu_relax();
+  }
+}
+
+
 void uv__async_close(uv_async_t* handle) {
+  uv__async_spin(handle);
   QUEUE_REMOVE(&handle->queue);
   uv__handle_stop(handle);
 }
@@ -109,8 +138,8 @@ static void uv__async_io(uv_loop_t* loop, uv__io_t* w, unsigned int events) {
     QUEUE_REMOVE(q);
     QUEUE_INSERT_TAIL(&loop->async_handles, q);
 
-    if (cmpxchgi(&h->pending, 1, 0) == 0)
-      continue;
+    if (0 == uv__async_spin(h))
+      continue;  /* Not pending. */
 
     if (h->async_cb == NULL)
       continue;