Remove the TAP-Win32 reader thread.
authorEtienne Dechamps <etienne@edechamps.fr>
Sat, 28 Jun 2014 17:39:00 +0000 (18:39 +0100)
committerEtienne Dechamps <etienne@edechamps.fr>
Sat, 28 Jun 2014 19:00:05 +0000 (20:00 +0100)
tinc is using a separate thread to read from the TAP device on Windows.
The rationale was that the notification mechanism for packets arriving
on the virtual network device is based on Win32 events, and the event
loop did not support listening to these events.

Thanks to recent improvements, this event loop limitation has been
lifted. Therefore we can get rid of the separate thread and simply add
the Win32 "incoming packet" event to the event loop, just like a socket.
The result is cleaner code that's easier to reason about.

src/event.c
src/event.h
src/mingw/device.c
src/net.h
src/tincd.c

index 8f15ebe..f349742 100644 (file)
@@ -294,9 +294,7 @@ bool event_loop(void) {
                DWORD timeout_ms = tv ? (tv->tv_sec * 1000 + tv->tv_usec / 1000 + 1) : WSA_INFINITE;
 
                if (!event_count) {
-                       LeaveCriticalSection(&mutex);
                        Sleep(timeout_ms);
-                       EnterCriticalSection(&mutex);
                        continue;
                }
 
@@ -328,9 +326,7 @@ bool event_loop(void) {
                        event_index++;
                }
 
-               LeaveCriticalSection(&mutex);
                DWORD result = WSAWaitForMultipleEvents(event_count, events, FALSE, timeout_ms, FALSE);
-               EnterCriticalSection(&mutex);
 
                WSAEVENT event;
                if (result >= WSA_WAIT_EVENT_0 && result < WSA_WAIT_EVENT_0 + event_count)
@@ -362,12 +358,6 @@ bool event_loop(void) {
        return true;
 }
 
-void event_flush_output(void) {
-       for splay_each(io_t, io, &io_tree)
-               if(io->flags & IO_WRITE)
-                       io->cb(io->data, IO_WRITE);
-}
-
 void event_exit(void) {
        running = false;
 }
index e4f5cee..0ff8e01 100644 (file)
@@ -71,7 +71,6 @@ extern void signal_add(signal_t *sig, signal_cb_t cb, void *data, int signum);
 extern void signal_del(signal_t *sig);
 
 extern bool event_loop(void);
-extern void event_flush_output(void);
 extern void event_exit(void);
 
 #endif
index a765ce3..33b13da 100644 (file)
@@ -36,6 +36,9 @@
 
 int device_fd = -1;
 static HANDLE device_handle = INVALID_HANDLE_VALUE;
+static io_t device_read_io;
+static OVERLAPPED device_read_overlapped;
+static vpn_packet_t device_read_packet;
 char *device = NULL;
 char *iface = NULL;
 static char *device_info = NULL;
@@ -45,44 +48,32 @@ static uint64_t device_total_out = 0;
 
 extern char *myport;
 
-static DWORD WINAPI tapreader(void *bla) {
-       int status;
-       DWORD len;
-       OVERLAPPED overlapped;
-       vpn_packet_t packet;
-
-       logger(DEBUG_ALWAYS, LOG_DEBUG, "Tap reader running");
-
-       /* Read from tap device and send to parent */
+static void device_issue_read() {
+       device_read_overlapped.Offset = 0;
+       device_read_overlapped.OffsetHigh = 0;
 
-       overlapped.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
+       int status = ReadFile(device_handle, (void *)device_read_packet.data, MTU, NULL, &device_read_overlapped);
 
-       for(;;) {
-               overlapped.Offset = 0;
-               overlapped.OffsetHigh = 0;
-               ResetEvent(overlapped.hEvent);
-
-               status = ReadFile(device_handle, (void *)packet.data, MTU, &len, &overlapped);
+       if(!status && GetLastError() != ERROR_IO_PENDING) {
+               logger(DEBUG_ALWAYS, LOG_ERR, "Error while reading from %s %s: %s", device_info,
+                          device, strerror(errno));
+       }
+}
 
-               if(!status) {
-                       if(GetLastError() == ERROR_IO_PENDING) {
-                               WaitForSingleObject(overlapped.hEvent, INFINITE);
-                               if(!GetOverlappedResult(device_handle, &overlapped, &len, FALSE))
-                                       continue;
-                       } else {
-                               logger(DEBUG_ALWAYS, LOG_ERR, "Error while reading from %s %s: %s", device_info,
-                                          device, strerror(errno));
-                               return -1;
-                       }
-               }
+static void device_handle_read(void *data) {
+       ResetEvent(device_read_overlapped.hEvent);
 
-               EnterCriticalSection(&mutex);
-               packet.len = len;
-               packet.priority = 0;
-               route(myself, &packet);
-               event_flush_output();
-               LeaveCriticalSection(&mutex);
+       DWORD len;
+       if (!GetOverlappedResult(device_handle, &device_read_overlapped, &len, FALSE)) {
+               logger(DEBUG_ALWAYS, LOG_ERR, "Error getting read result from %s %s: %s", device_info,
+                          device, strerror(errno));
+               return;
        }
+
+       device_read_packet.len = len;
+       device_read_packet.priority = 0;
+       route(myself, &device_read_packet);
+       device_issue_read();
 }
 
 static bool setup_device(void) {
@@ -192,12 +183,9 @@ static bool setup_device(void) {
 
        /* Start the tap reader */
 
-       thread = CreateThread(NULL, 0, tapreader, NULL, 0, NULL);
-
-       if(!thread) {
-               logger(DEBUG_ALWAYS, LOG_ERR, "System call `%s' failed: %s", "CreateThread", winerror(GetLastError()));
-               return false;
-       }
+       io_add_event(&device_read_io, device_handle_read, NULL, CreateEvent(NULL, TRUE, FALSE, NULL));
+       device_read_overlapped.hEvent = device_read_io.event;
+       device_issue_read();
 
        device_info = "Windows tap device";
 
@@ -221,6 +209,9 @@ static void disable_device(void) {
 }
 
 static void close_device(void) {
+       io_del(&device_read_io);
+       CancelIo(device_handle);
+       CloseHandle(device_read_overlapped.hEvent);
        CloseHandle(device_handle); device_handle = INVALID_HANDLE_VALUE;
 
        free(device); device = NULL;
index f0ece89..3b2cdf0 100644 (file)
--- a/src/net.h
+++ b/src/net.h
@@ -203,8 +203,6 @@ extern void load_all_nodes(void);
 
 #ifndef HAVE_MINGW
 #define closesocket(s) close(s)
-#else
-extern CRITICAL_SECTION mutex;
 #endif
 
 #endif /* __TINC_NET_H__ */
index 56ed2f0..87c0b7d 100644 (file)
@@ -106,7 +106,6 @@ static struct option const long_options[] = {
 
 #ifdef HAVE_MINGW
 static struct WSAData wsa_state;
-CRITICAL_SECTION mutex;
 int main2(int argc, char **argv);
 #endif
 
@@ -378,8 +377,6 @@ int main(int argc, char **argv) {
 }
 
 int main2(int argc, char **argv) {
-       InitializeCriticalSection(&mutex);
-       EnterCriticalSection(&mutex);
 #endif
        char *priority = NULL;