Call WSAWaitForMultipleEvents() in a loop until we have checked all events.
authorTodd C. Miller <Todd.Miller@sudo.ws>
Tue, 27 Feb 2018 21:20:46 +0000 (14:20 -0700)
committerGuus Sliepen <guus@tinc-vpn.org>
Wed, 28 Feb 2018 20:20:58 +0000 (21:20 +0100)
WSAWaitForMultipleEvents() only returns the index of the first event that is read.  We need to call WSAWaitForMultipleEvents() repeatedly to check if other events are also ready.  Otherwise, a single busy event (such as the TAP device) can starve the other events.

src/event.c
src/splay_tree.c
src/splay_tree.h

index 331872a..9cd7d07 100644 (file)
@@ -389,71 +389,87 @@ bool event_loop(void) {
                   Note that technically FD_CLOSE has the same problem, but it's okay because user code does not rely on
                   this event being fired again if ignored.
                */
-               io_t *writeable_io = NULL;
+               unsigned int curgen = io_tree.generation;
 
-               for splay_each(io_t, io, &io_tree)
+               for splay_each(io_t, io, &io_tree) {
                        if(io->flags & IO_WRITE && send(io->fd, NULL, 0, 0) == 0) {
-                               writeable_io = io;
-                               break;
+                               io->cb(io->data, IO_WRITE);
+
+                               if(curgen != io_tree.generation) {
+                                       break;
+                               }
                        }
+               }
 
-               if(writeable_io) {
-                       writeable_io->cb(writeable_io->data, IO_WRITE);
-                       continue;
+               if(event_count > WSA_MAXIMUM_WAIT_EVENTS) {
+                       WSASetLastError(WSA_INVALID_PARAMETER);
+                       return(false);
                }
 
-               WSAEVENT *events = xmalloc(event_count * sizeof(*events));
+               WSAEVENT events[WSA_MAXIMUM_WAIT_EVENTS];
+               io_t *io_map[WSA_MAXIMUM_WAIT_EVENTS];
                DWORD event_index = 0;
 
                for splay_each(io_t, io, &io_tree) {
                        events[event_index] = io->event;
+                       io_map[event_index] = io;
                        event_index++;
                }
 
-               DWORD result = WSAWaitForMultipleEvents(event_count, events, FALSE, timeout_ms, FALSE);
+               /*
+                * If the generation number changes due to event addition
+                * or removal by a callback we restart the loop.
+                */
+               curgen = io_tree.generation;
 
-               WSAEVENT event;
+               for(DWORD event_offset = 0; event_offset < event_count;) {
+                       DWORD result = WSAWaitForMultipleEvents(event_count - event_offset, &events[event_offset], FALSE, timeout_ms, FALSE);
 
-               if(result >= WSA_WAIT_EVENT_0 && result < WSA_WAIT_EVENT_0 + event_count) {
-                       event = events[result - WSA_WAIT_EVENT_0];
-               }
+                       if(result == WSA_WAIT_TIMEOUT) {
+                               break;
+                       }
 
-               free(events);
+                       if(result < WSA_WAIT_EVENT_0 || result >= WSA_WAIT_EVENT_0 + event_count - event_offset) {
+                               return(false);
+                       }
 
-               if(result == WSA_WAIT_TIMEOUT) {
-                       continue;
-               }
+                       /* Look up io in the map by index. */
+                       event_index = result - WSA_WAIT_EVENT_0 + event_offset;
+                       io_t *io = io_map[event_index];
 
-               if(result < WSA_WAIT_EVENT_0 || result >= WSA_WAIT_EVENT_0 + event_count) {
-                       return false;
-               }
+                       if(io->fd == -1) {
+                               io->cb(io->data, 0);
 
-               io_t *io = splay_search(&io_tree, &((io_t) {
-                       .event = event
-               }));
+                               if(curgen != io_tree.generation) {
+                                       break;
+                               }
+                       } else {
+                               WSANETWORKEVENTS network_events;
 
-               if(!io) {
-                       abort();
-               }
+                               if(WSAEnumNetworkEvents(io->fd, io->event, &network_events) != 0) {
+                                       return(false);
+                               }
 
-               if(io->fd == -1) {
-                       io->cb(io->data, 0);
-               } else {
-                       WSANETWORKEVENTS network_events;
+                               if(network_events.lNetworkEvents & READ_EVENTS) {
+                                       io->cb(io->data, IO_READ);
 
-                       if(WSAEnumNetworkEvents(io->fd, io->event, &network_events) != 0) {
-                               return false;
-                       }
+                                       if(curgen != io_tree.generation) {
+                                               break;
+                                       }
+                               }
 
-                       if(network_events.lNetworkEvents & READ_EVENTS) {
-                               io->cb(io->data, IO_READ);
+                               /*
+                                   The fd might be available for write too. However, if we already fired the read callback, that
+                                   callback might have deleted the io (e.g. through terminate_connection()), so we can't fire the
+                                   write callback here. Instead, we loop back and let the writable io loop above handle it.
+                                */
                        }
 
-                       /*
-                           The fd might be available for write too. However, if we already fired the read callback, that
-                           callback might have deleted the io (e.g. through terminate_connection()), so we can't fire the
-                           write callback here. Instead, we loop back and let the writable io loop above handle it.
-                        */
+                       /* Continue checking the rest of the events. */
+                       event_offset = event_index + 1;
+
+                       /* Just poll the next time through. */
+                       timeout_ms = 0;
                }
        }
 
index 2b6186f..ee85519 100644 (file)
@@ -463,6 +463,7 @@ void splay_insert_top(splay_tree_t *tree, splay_node_t *node) {
        node->prev = node->next = node->left = node->right = node->parent = NULL;
        tree->head = tree->tail = tree->root = node;
        tree->count++;
+       tree->generation++;
 }
 
 void splay_insert_before(splay_tree_t *tree, splay_node_t *before, splay_node_t *node) {
@@ -500,6 +501,7 @@ void splay_insert_before(splay_tree_t *tree, splay_node_t *before, splay_node_t
        node->parent = NULL;
        tree->root = node;
        tree->count++;
+       tree->generation++;
 }
 
 void splay_insert_after(splay_tree_t *tree, splay_node_t *after, splay_node_t *node) {
@@ -537,6 +539,7 @@ void splay_insert_after(splay_tree_t *tree, splay_node_t *after, splay_node_t *n
        node->parent = NULL;
        tree->root = node;
        tree->count++;
+       tree->generation++;
 }
 
 splay_node_t *splay_unlink(splay_tree_t *tree, void *data) {
@@ -581,6 +584,7 @@ void splay_unlink_node(splay_tree_t *tree, splay_node_t *node) {
        }
 
        tree->count--;
+       tree->generation++;
 }
 
 void splay_delete_node(splay_tree_t *tree, splay_node_t *node) {
index 06367bf..d5ab742 100644 (file)
@@ -57,7 +57,8 @@ typedef struct splay_tree_t {
        splay_compare_t compare;
        splay_action_t delete;
 
-       int count;
+       unsigned int count;
+       unsigned int generation;
 
 } splay_tree_t;