luo980/libuv
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

unix: fix for uv_async data race

Browse Source 
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/197
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-11 14:47:20 -07:00 committed by Ben Noordhuis
parent 91d3598475
commit 22725f2433
2 changed files with 25 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
src/unix/async.c
View File

@ -24,6 +24,7 @@
#include "uv.h"
#include "internal.h"
#include "atomic-ops.h"
#include <errno.h>
#include <assert.h>
@ -34,7 +35,6 @@
static void uv__async_event(uv_loop_t* loop,
struct uv__async* w,
unsigned int nevents);
static int uv__async_make_pending(int* pending);
static int uv__async_eventfd(void);
@ -54,7 +54,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) {
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);
return 0;
@ -75,44 +79,15 @@ static void uv__async_event(uv_loop_t* loop,
ngx_queue_foreach(q, &loop->async_handles) {
h = ngx_queue_data(q, uv_async_t, queue);
if (!h->pending) continue;
h->pending = 0;
if (cmpxchgi(&h->pending, 1, 0) == 0)
continue;
h->async_cb(h, 0);
}
}
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) {
struct uv__async* wa;
char buf[1024];

15
src/unix/internal.h
View File

@ -66,6 +66,21 @@
} \
while (0)
/* The __clang__ and __INTEL_COMPILER checks are superfluous because they
* define __GNUC__. They are here to convey to you, dear reader, that these
* macros are enabled when compiling with clang or icc.
*/
#if defined(__clang__) || \
defined(__GNUC__) || \
defined(__INTEL_COMPILER) || \
defined(__SUNPRO_C)
# define UV_DESTRUCTOR(declaration) __attribute__((destructor)) declaration
# define UV_UNUSED(declaration) __attribute__((unused)) declaration
#else
# define UV_DESTRUCTOR(declaration) declaration
# define UV_UNUSED(declaration) declaration
#endif
#if defined(__linux__)
# define UV__POLLIN UV__EPOLLIN
# define UV__POLLOUT UV__EPOLLOUT