From 1948fc273c4f4b78f90c8ca8fbc22647e30f431c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Chigot?= <chigot@adacore.com>
Date: Wed, 24 Aug 2022 15:42:33 +0200
Subject: [PATCH] io: switch completely to Windows API for socket watch on
 win32

The current implementation is using a mixed of select and the Windows
API (WSASelectEvent, etc).
As select doesn't clear the Windows events, WSAENumNetworkEvents must be
called before select to do it.
However, as this operation isn't made atomically, some events might be
reset before being retrieved by select. Resulting in them being skipped
by the polling.

In order to avoid this issue, this patch improves socket watch to be
based solely on Windows API.

Note that because several GSources might be created for the same HANDLE,
this implementation has no way to know on which GSources an event must be
returned. However, it seems that only two GSources are created, a master
one with just "G_IO_HUP" and the child one which aims to retrieve all
the events (ie with "G_IO_IN", "G_IO_OUT", etc). So, to avoid the master
GSource receiving events meant for the child, we aren't calling
WSAEnumNetworkEvents for it (or for any GSource having only G_IO_HUP as
condition). As G_IO_HUP was never returned by the previous, I supposed
that it should be safe to do so.

TN: V812-020
Change-Id: I037fbe93e0641894b0096c39ace5d0d68db8e2e3
---
 io/channel-socket.c |  2 +-
 io/channel-watch.c  | 57 +++++++++++++++++++++++----------------------
 2 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/io/channel-socket.c b/io/channel-socket.c
index b76dca9cc1..75500e5647 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -69,7 +69,7 @@ qio_channel_socket_new(void)
     qio_channel_set_feature(ioc, QIO_CHANNEL_FEATURE_SHUTDOWN);
 
 #ifdef WIN32
-    ioc->event = CreateEvent(NULL, FALSE, FALSE, NULL);
+    ioc->event = WSACreateEvent();
 #endif
 
     trace_qio_channel_socket_new(sioc);
diff --git a/io/channel-watch.c b/io/channel-watch.c
index 0289b3647c..9c9aca41f5 100644
--- a/io/channel-watch.c
+++ b/io/channel-watch.c
@@ -114,46 +114,39 @@ qio_channel_socket_source_prepare(GSource *source G_GNUC_UNUSED,
 static gboolean
 qio_channel_socket_source_check(GSource *source)
 {
-    static struct timeval tv0;
-
     QIOChannelSocketSource *ssource = (QIOChannelSocketSource *)source;
     WSANETWORKEVENTS ev;
-    fd_set rfds, wfds, xfds;
 
     if (!ssource->condition) {
         return 0;
     }
 
-    WSAEnumNetworkEvents(ssource->socket, ssource->ioc->event, &ev);
-
-    FD_ZERO(&rfds);
-    FD_ZERO(&wfds);
-    FD_ZERO(&xfds);
-    if (ssource->condition & G_IO_IN) {
-        FD_SET((SOCKET)ssource->socket, &rfds);
-    }
-    if (ssource->condition & G_IO_OUT) {
-        FD_SET((SOCKET)ssource->socket, &wfds);
-    }
-    if (ssource->condition & G_IO_PRI) {
-        FD_SET((SOCKET)ssource->socket, &xfds);
+    /* For now, we don't support G_IO_HUP checks.
+       We want to avoid calling WSAEnumNetworkEvents for any GSource
+       having just G_IO_HUP. It might hide events aimed to be retrieved by
+       other GSources waiting inputs or outputs (ie with G_IO_IN or G_IO_OUT).
+       The reason is that the Windows API is based on HANDLE but we often
+       create several GSources for the same HANDLE. Thus, input events might
+       be picked and cleared by the G_IO_HUP GSource.  */
+    if (ssource->condition == G_IO_HUP) {
+        return 0;
     }
-    ssource->revents = 0;
-    if (select(0, &rfds, &wfds, &xfds, &tv0) == 0) {
+
+    if (WSAEnumNetworkEvents(ssource->socket, ssource->ioc->event, &ev)) {
         return 0;
     }
 
-    if (FD_ISSET(ssource->socket, &rfds)) {
+    ssource->revents = 0;
+
+    if (ev.lNetworkEvents & (FD_READ | FD_ACCEPT | FD_OOB)) {
         ssource->revents |= G_IO_IN;
     }
-    if (FD_ISSET(ssource->socket, &wfds)) {
+
+    if (ev.lNetworkEvents & (FD_WRITE | FD_CONNECT)) {
         ssource->revents |= G_IO_OUT;
     }
-    if (FD_ISSET(ssource->socket, &xfds)) {
-        ssource->revents |= G_IO_PRI;
-    }
 
-    return ssource->revents;
+    return ssource->revents & ssource->condition;
 }
 
 
@@ -174,6 +167,7 @@ qio_channel_socket_source_finalize(GSource *source)
 {
     QIOChannelSocketSource *ssource = (QIOChannelSocketSource *)source;
 
+
     object_unref(OBJECT(ssource->ioc));
 }
 
@@ -286,9 +280,16 @@ GSource *qio_channel_create_socket_watch(QIOChannel *ioc,
     QIOChannelSocketSource *ssource;
 
 #ifdef WIN32
-    WSAEventSelect(socket, ioc->event,
-                   FD_READ | FD_ACCEPT | FD_CLOSE |
-                   FD_CONNECT | FD_WRITE | FD_OOB);
+    int ev = 0;
+
+    if (condition & G_IO_IN) {
+        ev |= (FD_READ | FD_ACCEPT | FD_OOB);
+    }
+    if (condition & G_IO_OUT) {
+        ev |= (FD_WRITE | FD_CONNECT);
+    }
+
+    WSAEventSelect(socket, ioc->event, ev);
 #endif
 
     source = g_source_new(&qio_channel_socket_source_funcs,
@@ -303,7 +304,7 @@ GSource *qio_channel_create_socket_watch(QIOChannel *ioc,
     ssource->revents = 0;
 
     ssource->fd.fd = (gintptr)ioc->event;
-    ssource->fd.events = G_IO_IN;
+    ssource->fd.events = condition;
 
     g_source_add_poll(source, &ssource->fd);
 
-- 
2.25.1