unix: fix for uv_async data race

There's a data race in the consuming side of uv_async. The "pending"
flag could be trampled by producing thread causing an async send
event to be missed.

PR-URL: https://github.com/libuv/libuv/pull/189
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
This commit is contained in:
Michael Penick 2015-02-17 17:22:11 -07:00 committed by Ben Noordhuis
parent 672b204799
commit 4ed237278c

View File

@ -24,6 +24,7 @@
#include "uv.h" #include "uv.h"
#include "internal.h" #include "internal.h"
#include "atomic-ops.h"
#include <errno.h> #include <errno.h>
#include <stdio.h> /* snprintf() */ #include <stdio.h> /* snprintf() */
@ -35,7 +36,6 @@
static void uv__async_event(uv_loop_t* loop, static void uv__async_event(uv_loop_t* loop,
struct uv__async* w, struct uv__async* w,
unsigned int nevents); unsigned int nevents);
static int uv__async_make_pending(int* pending);
static int uv__async_eventfd(void); static int uv__async_eventfd(void);
@ -58,7 +58,11 @@ int uv_async_init(uv_loop_t* loop, uv_async_t* handle, uv_async_cb async_cb) {
int uv_async_send(uv_async_t* handle) { int uv_async_send(uv_async_t* handle) {
if (uv__async_make_pending(&handle->pending) == 0) /* Do a cheap read first. */
if (ACCESS_ONCE(int, handle->pending) != 0)
return 0;
if (cmpxchgi(&handle->pending, 0, 1) == 0)
uv__async_send(&handle->loop->async_watcher); uv__async_send(&handle->loop->async_watcher);
return 0; return 0;
@ -80,9 +84,8 @@ static void uv__async_event(uv_loop_t* loop,
QUEUE_FOREACH(q, &loop->async_handles) { QUEUE_FOREACH(q, &loop->async_handles) {
h = QUEUE_DATA(q, uv_async_t, queue); h = QUEUE_DATA(q, uv_async_t, queue);
if (h->pending == 0) if (cmpxchgi(&h->pending, 1, 0) == 0)
continue; continue;
h->pending = 0;
if (h->async_cb == NULL) if (h->async_cb == NULL)
continue; continue;
@ -91,37 +94,6 @@ static void uv__async_event(uv_loop_t* loop,
} }
static int uv__async_make_pending(int* pending) {
/* Do a cheap read first. */
if (ACCESS_ONCE(int, *pending) != 0)
return 1;
/* Micro-optimization: use atomic memory operations to detect if we've been
* preempted by another thread and don't have to make an expensive syscall.
* This speeds up the heavily contended case by about 1-2% and has little
* if any impact on the non-contended case.
*
* Use XCHG instead of the CMPXCHG that __sync_val_compare_and_swap() emits
* on x86, it's about 4x faster. It probably makes zero difference in the
* grand scheme of things but I'm OCD enough not to let this one pass.
*/
#if defined(__i386__) || defined(__x86_64__)
{
unsigned int val = 1;
__asm__ __volatile__ ("xchgl %0, %1"
: "+r" (val)
: "m" (*pending));
return val != 0;
}
#elif defined(__GNUC__) && (__GNUC__ > 4 || __GNUC__ == 4 && __GNUC_MINOR__ > 0)
return __sync_val_compare_and_swap(pending, 0, 1) != 0;
#else
ACCESS_ONCE(int, *pending) = 1;
return 0;
#endif
}
static void uv__async_io(uv_loop_t* loop, uv__io_t* w, unsigned int events) { static void uv__async_io(uv_loop_t* loop, uv__io_t* w, unsigned int events) {
struct uv__async* wa; struct uv__async* wa;
char buf[1024]; char buf[1024];