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

win,async: refactor internal implementation

Browse Source 
- Use atomic ops both to set and reset async_sent flag
- Remove the MinGW atomic ops, since Windows intrinsics are supported
- Remone thread-unsafe checks from uv_async_send

According to MSDN there are no alignment requirements. We could use
InterlockedExchange8, but that's only available on Windows >= 8.

This change is intended to make uv_async_send more resilient. It has to
be thread-safe, and that means that the handle could just have been
closed when uv_async_send was called. This case was previously not
handles (there is an inherent race condition). The new model is inspired
by the one used on the Unix side, which uses a single fd (or overlapped
in this case) to wakeup the loop and then process all pending async
handles. This makes handling those edge cases a lot simpler: when the
handle is closed it's removed from the handle queue, and then it's not
processed at all.

As a result of this change, async benchmarks work on Windows where they
previously failed with assertions.

PR-URL: https://github.com/libuv/libuv/pull/980
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit is contained in:
Saúl Ibarra Corretgé 2016-08-05 15:44:39 +01:00
parent 7892bd6f76
commit 78a1cd167f
9 changed files with 42 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Makefile.am
View File

@ -47,7 +47,6 @@ AM_CPPFLAGS += -I$(top_srcdir)/src/win \
-DWIN32_LEAN_AND_MEAN \
-D_WIN32_WINNT=0x0600
libuv_la_SOURCES += src/win/async.c \
src/win/atomicops-inl.h \
src/win/core.c \
src/win/dl.c \
src/win/error.c \

1
Makefile.mingw
View File

@ -32,7 +32,6 @@ INCLUDES = include/tree.h \
src/heap-inl.h \
src/queue.h \
src/uv-common.h \
src/win/atomicops-inl.h \
src/win/handle-inl.h \
src/win/internal.h \
src/win/req-inl.h \

10
include/uv-win.h
View File

@ -310,7 +310,10 @@ RB_HEAD(uv_timer_tree_s, uv_timer_s);
/* Threadpool */ \
void* wq[2]; \
uv_mutex_t wq_mutex; \
uv_async_t wq_async;
uv_async_t wq_async; \
/* Async handle */ \
struct uv_req_s async_req; \
void* async_handles[2];
#define UV_REQ_TYPE_PRIVATE \
/* TODO: remove the req suffix */ \
@ -498,10 +501,9 @@ RB_HEAD(uv_timer_tree_s, uv_timer_s);
uv_timer_cb timer_cb;
#define UV_ASYNC_PRIVATE_FIELDS \
struct uv_req_s async_req; \
void* queue[2]; \
uv_async_cb async_cb; \
/* char to avoid alignment issues */ \
char volatile async_sent;
LONG volatile async_sent;
#define UV_PREPARE_PRIVATE_FIELDS \
void* queue[2]; \

67
src/win/async.c
View File

@ -23,32 +23,23 @@
#include "uv.h"
#include "internal.h"
#include "atomicops-inl.h"
#include "handle-inl.h"
#include "req-inl.h"
void uv_async_endgame(uv_loop_t* loop, uv_async_t* handle) {
if (handle->flags & UV__HANDLE_CLOSING &&
!handle->async_sent) {
assert(!(handle->flags & UV_HANDLE_CLOSED));
uv__handle_close(handle);
}
assert(handle->flags & UV__HANDLE_CLOSING);
assert(!(handle->flags & UV_HANDLE_CLOSED));
uv__handle_close(handle);
}
int uv_async_init(uv_loop_t* loop, uv_async_t* handle, uv_async_cb async_cb) {
uv_req_t* req;
uv__handle_init(loop, (uv_handle_t*) handle, UV_ASYNC);
handle->async_sent = 0;
handle->async_cb = async_cb;
req = &handle->async_req;
uv_req_init(loop, req);
req->type = UV_WAKEUP;
req->data = handle;
QUEUE_INSERT_TAIL(&loop->async_handles, &handle->queue);
uv__handle_start(handle);
return 0;
@ -56,44 +47,46 @@ int uv_async_init(uv_loop_t* loop, uv_async_t* handle, uv_async_cb async_cb) {
void uv_async_close(uv_loop_t* loop, uv_async_t* handle) {
if (!((uv_async_t*)handle)->async_sent) {
uv_want_endgame(loop, (uv_handle_t*) handle);
}
QUEUE_REMOVE(&handle->queue);
uv_want_endgame(loop, (uv_handle_t*) handle);
uv__handle_closing(handle);
}
int uv_async_send(uv_async_t* handle) {
uv_loop_t* loop = handle->loop;
/* First do a cheap read. */
if (handle->async_sent != 0)
return 0;
if (handle->type != UV_ASYNC) {
/* Can't set errno because that's not thread-safe. */
return -1;
}
/* The user should make sure never to call uv_async_send to a closing */
/* or closed handle. */
assert(!(handle->flags & UV__HANDLE_CLOSING));
if (!uv__atomic_exchange_set(&handle->async_sent)) {
POST_COMPLETION_FOR_REQ(loop, &handle->async_req);
if (InterlockedExchange(&handle->async_sent, 1) == 0) {
uv_loop_t* loop = handle->loop;
POST_COMPLETION_FOR_REQ(loop, &loop->async_req);
}
return 0;
}
void uv_process_async_wakeup_req(uv_loop_t* loop, uv_async_t* handle,
uv_req_t* req) {
assert(handle->type == UV_ASYNC);
void uv_process_async_wakeup_req(uv_loop_t* loop,
uv_req_t* req) {
assert(req->type == UV_WAKEUP);
handle->async_sent = 0;
QUEUE queue;
QUEUE* q;
uv_async_t* h;
if (handle->flags & UV__HANDLE_CLOSING) {
uv_want_endgame(loop, (uv_handle_t*)handle);
} else if (handle->async_cb != NULL) {
handle->async_cb(handle);
QUEUE_MOVE(&loop->async_handles, &queue);
while (!QUEUE_EMPTY(&queue)) {
q = QUEUE_HEAD(&queue);
h = QUEUE_DATA(q, uv_async_t, queue);
QUEUE_REMOVE(q);
QUEUE_INSERT_TAIL(&loop->async_handles, q);
if (InterlockedExchange(&h->async_sent, 0) == 0)
continue;
if (h->async_cb != NULL)
h->async_cb(h);
}
}

56
src/win/atomicops-inl.h
View File

@ -1,56 +0,0 @@
/* Copyright Joyent, Inc. and other Node contributors. All rights reserved.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
* deal in the Software without restriction, including without limitation the
* rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
* sell copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
* IN THE SOFTWARE.
*/
#ifndef UV_WIN_ATOMICOPS_INL_H_
#define UV_WIN_ATOMICOPS_INL_H_
#include "uv.h"
/* Atomic set operation on char */
#ifdef _MSC_VER /* MSVC */
/* _InterlockedOr8 is supported by MSVC on x32 and x64. It is slightly less */
/* efficient than InterlockedExchange, but InterlockedExchange8 does not */
/* exist, and interlocked operations on larger targets might require the */
/* target to be aligned. */
#pragma intrinsic(_InterlockedOr8)
static char __declspec(inline) uv__atomic_exchange_set(char volatile* target) {
return _InterlockedOr8(target, 1);
}
#else /* GCC */
/* Mingw-32 version, hopefully this works for 64-bit gcc as well. */
static inline char uv__atomic_exchange_set(char volatile* target) {
const char one = 1;
char old_value;
__asm__ __volatile__ ("lock xchgb %0, %1\n\t"
: "=r"(old_value), "=m"(*target)
: "0"(one), "m"(*target)
: "memory");
return old_value;
}
#endif
#endif /* UV_WIN_ATOMICOPS_INL_H_ */

4
src/win/core.c
View File

@ -153,6 +153,10 @@ int uv_loop_init(uv_loop_t* loop) {
QUEUE_INIT(&loop->prepare_handles);
QUEUE_INIT(&loop->idle_handles);
QUEUE_INIT(&loop->async_handles);
uv_req_init(loop, &loop->async_req);
loop->async_req.type = UV_WAKEUP;
memset(&loop->poll_peer_sockets, 0, sizeof loop->poll_peer_sockets);
loop->timer_counter = 0;

3
src/win/internal.h
View File

@ -267,8 +267,7 @@ void uv__once_init();
void uv_async_close(uv_loop_t* loop, uv_async_t* handle);
void uv_async_endgame(uv_loop_t* loop, uv_async_t* handle);
void uv_process_async_wakeup_req(uv_loop_t* loop, uv_async_t* handle,
uv_req_t* req);
void uv_process_async_wakeup_req(uv_loop_t* loop, uv_req_t* req);
/*

2
src/win/req-inl.h
View File

@ -194,7 +194,7 @@ INLINE static int uv_process_reqs(uv_loop_t* loop) {
break;
case UV_WAKEUP:
uv_process_async_wakeup_req(loop, (uv_async_t*) req->data, req);
uv_process_async_wakeup_req(loop, req);
break;
case UV_SIGNAL_REQ:

1
uv.gyp
View File

@ -74,7 +74,6 @@
'sources': [
'include/uv-win.h',
'src/win/async.c',
'src/win/atomicops-inl.h',
'src/win/core.c',
'src/win/dl.c',
'src/win/error.c',