From: Todd C. Miller <Todd.Miller@sudo.ws>
Date: Tue, 27 Feb 2018 21:20:46 +0000 (-0700)
Subject: Call WSAWaitForMultipleEvents() in a loop until we have checked all events.
X-Git-Tag: release-1.1pre16~15
X-Git-Url: https://git.tinc-vpn.org/git/browse?a=commitdiff_plain;h=d6c8a1a3d3e945142b251b2897517e10ce0dfce4;p=tinc

Call WSAWaitForMultipleEvents() in a loop until we have checked all events.
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.
---

diff --git a/src/event.c b/src/event.c
index 331872a5..9cd7d07a 100644
--- a/src/event.c
+++ b/src/event.c
@@ -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;
 		}
 	}
 
diff --git a/src/splay_tree.c b/src/splay_tree.c
index 2b6186ff..ee855192 100644
--- a/src/splay_tree.c
+++ b/src/splay_tree.c
@@ -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) {
diff --git a/src/splay_tree.h b/src/splay_tree.h
index 06367bf5..d5ab7421 100644
--- a/src/splay_tree.h
+++ b/src/splay_tree.h
@@ -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;