unix: use QUEUE_MOVE when iterating over lists

Replace uses of QUEUE_FOREACH when the list can get modified while
iterating over it, in particular when a callback is made into the
user's code.  This should fix a number of spurious failures that
people have been reporting.

PR-URL: https://github.com/libuv/libuv/pull/565
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
This commit is contained in:
Ben Noordhuis 2015-10-08 17:03:04 +02:00
parent 1867a6c1ce
commit 442b8a5a84
6 changed files with 34 additions and 4 deletions

View File

@ -30,6 +30,9 @@ typedef void *QUEUE[2];
#define QUEUE_DATA(ptr, type, field) \
((type *) ((char *) (ptr) - offsetof(type, field)))
/* Important note: mutating the list while QUEUE_FOREACH is
* iterating over its elements results in undefined behavior.
*/
#define QUEUE_FOREACH(q, h) \
for ((q) = QUEUE_NEXT(h); (q) != (h); (q) = QUEUE_NEXT(q))

View File

@ -78,12 +78,18 @@ void uv__async_close(uv_async_t* handle) {
static void uv__async_event(uv_loop_t* loop,
struct uv__async* w,
unsigned int nevents) {
QUEUE queue;
QUEUE* q;
uv_async_t* h;
QUEUE_FOREACH(q, &loop->async_handles) {
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 (cmpxchgi(&h->pending, 1, 0) == 0)
continue;

View File

@ -120,6 +120,7 @@ static void uv__inotify_read(uv_loop_t* loop,
const struct uv__inotify_event* e;
struct watcher_list* w;
uv_fs_event_t* h;
QUEUE queue;
QUEUE* q;
const char* path;
ssize_t size;
@ -159,8 +160,14 @@ static void uv__inotify_read(uv_loop_t* loop,
*/
path = e->len ? (const char*) (e + 1) : uv__basename_r(w->path);
QUEUE_FOREACH(q, &w->watchers) {
QUEUE_MOVE(&w->watchers, &queue);
while (!QUEUE_EMPTY(&queue)) {
q = QUEUE_HEAD(&queue);
h = QUEUE_DATA(q, uv_fs_event_t, watchers);
QUEUE_REMOVE(q);
QUEUE_INSERT_TAIL(&w->watchers, q);
h->cb(h, path, events, 0);
}
}

View File

@ -47,9 +47,14 @@
\
void uv__run_##name(uv_loop_t* loop) { \
uv_##name##_t* h; \
QUEUE queue; \
QUEUE* q; \
QUEUE_FOREACH(q, &loop->name##_handles) { \
QUEUE_MOVE(&loop->name##_handles, &queue); \
while (!QUEUE_EMPTY(&queue)) { \
q = QUEUE_HEAD(&queue); \
h = QUEUE_DATA(q, uv_##name##_t, queue); \
QUEUE_REMOVE(q); \
QUEUE_INSERT_TAIL(&loop->name##_handles, q); \
h->name##_cb(h); \
} \
} \

View File

@ -234,6 +234,8 @@ void uv__signal_loop_cleanup(uv_loop_t* loop) {
/* Stop all the signal watchers that are still attached to this loop. This
* ensures that the (shared) signal tree doesn't contain any invalid entries
* entries, and that signal handlers are removed when appropriate.
* It's safe to use QUEUE_FOREACH here because the handles and the handle
* queue are not modified by uv__signal_stop().
*/
QUEUE_FOREACH(q, &loop->handle_queue) {
uv_handle_t* handle = QUEUE_DATA(q, uv_handle_t, handle_queue);

View File

@ -337,11 +337,18 @@ int uv_udp_recv_stop(uv_udp_t* handle) {
void uv_walk(uv_loop_t* loop, uv_walk_cb walk_cb, void* arg) {
QUEUE queue;
QUEUE* q;
uv_handle_t* h;
QUEUE_FOREACH(q, &loop->handle_queue) {
QUEUE_MOVE(&loop->handle_queue, &queue);
while (!QUEUE_EMPTY(&queue)) {
q = QUEUE_HEAD(&queue);
h = QUEUE_DATA(q, uv_handle_t, handle_queue);
QUEUE_REMOVE(q);
QUEUE_INSERT_TAIL(&loop->handle_queue, q);
if (h->flags & UV__HANDLE_INTERNAL) continue;
walk_cb(h, arg);
}