bug#45821: Emacs UDP support on Windows

[Alex Matei <matei.alexandru@live.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 5910 bytes --]

Thanks for the tips with logging!

I think I found the fix (at least for one of the issues?)
Please see the attached patch/diff for the fix.

The code will always  ignore (aka ‘eat’) one character if the file descriptor had the least significant bit set (0x1, so this would mean for any file descriptor with READ flag set)
[cid:image004.png@01D91EBA.5498F140]

Based on operator precedence rules, FILE_SOCKET == 0 will be evaluated first (before &) , resulting in 0, which would effectively cause the condition to always be false

This behavior can be observed when executing a call to  ‘(async-shell-command)’ (-> file descriptor flags 0x191) on a build with the patch vs one without the patch applied (see below, attached image)

  *   My suspicion is that this behavior is similar to what TLS was experiencing but just easier to reproduce (note: there might other issues with TLS , but so far, after applying this fix I have not run into any other issues)

[cid:image005.png@01D91EBA.5498F140]


@Robert Pluim<mailto:rpluim@gmail.com> I don’t know if this was the only missing piece, but from all my tests I couldn’t see any issues with TLS anymore, and with navigating https://gnu.org in EWW..
I wanted to thank you for creating this patch 🙏, and giving me pointers on how to apply and debug it.

In the meantime, I will continue to selfhost EWW and see if there are still other issues lurking around beyond this fix.


Funny enough, I ended up with good old ‘printf’, since it was nicely integrated with my build workflow: I would build and then just run emacs.exe from the shell => all my logging was nicely displayed there. ( message1 is also super nice, especially for useful user messages).


p.s.
- The patch also adds STATUS_READ_IN_PROGRESS state for the new ‘_sys_wait_readable’ function , based on what the read ahead logic was doing already (idk if it is needed, but it is a nice mirror, and a good status code to reflect, although I am not convinced anyone cares about this state transition..)
- I checked in other places of the code if we have issues with operator precedence for comparing socket flags, etc. and we are safe, most probably because we don’t bother with explicit equality conditions (like == 0) and instead we use the short form `if ( flag & socket_flag)`)

Sent from Mail<https://go.microsoft.com/fwlink/?LinkId=550986> for Windows

From: Eli Zaretskii<mailto:eliz@gnu.org>
Sent: Monday, January 2, 2023 5:38 AM
To: Robert Pluim<mailto:rpluim@gmail.com>
Cc: matei.alexandru@live.com<mailto:matei.alexandru@live.com>; 45821@debbugs.gnu.org<mailto:45821@debbugs.gnu.org>
Subject: Re: bug#45821: Emacs UDP support on Windows

> From: Robert Pluim <rpluim@gmail.com>
> Cc: matei.alexandru@live.com,  45821@debbugs.gnu.org
> Date: Mon, 02 Jan 2023 14:29:50 +0100
>
> >>>>> On Mon, 02 Jan 2023 14:41:00 +0200, Eli Zaretskii <eliz@gnu.org> said:
>
>     >> Cc: "45821@debbugs.gnu.org" <45821@debbugs.gnu.org>
>     >> From: Robert Pluim <rpluim@gmail.com>
>     >> Date: Mon, 02 Jan 2023 11:22:03 +0100
>     >>
>     Alex> ❌ Indeed, TLS is broken -> Eww to
>     >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.gnu.org%2F&data=05%7C01%7C%7Cb554d88b24e74c4839a108daecc69c01%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638082635001245924%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2FqTmb2XqqC%2B9k7jFDvImdYlbUMXMJKCWJ3sNSV7hJVE%3D&reserved=0<https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.gnu.org%2F&data=05%7C01%7C%7Cb554d88b24e74c4839a108daecc69c01%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638082635001245924%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2FqTmb2XqqC%2B9k7jFDvImdYlbUMXMJKCWJ3sNSV7hJVE%3D&reserved=0><https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.gnu.org%2F&data=05%7C01%7C%7Cb554d88b24e74c4839a108daecc69c01%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638082635001245924%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2FqTmb2XqqC%2B9k7jFDvImdYlbUMXMJKCWJ3sNSV7hJVE%3D&reserved=0%3chttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.gnu.org%2F&data=05%7C01%7C%7Cb554d88b24e74c4839a108daecc69c01%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638082635001245924%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2FqTmb2XqqC%2B9k7jFDvImdYlbUMXMJKCWJ3sNSV7hJVE%3D&reserved=0%3e> fails to load the page (
>     >> see attached image – Emacs instance on the left, compiled with UDP
>     >> patch, didn’t load gnu.org while on the right side- default Emacs
>     >> build for 28.1 opens it without any issues)
>     >>
>     >> Yep. Last time I looked at this, the TLS handshaking fails to complete
>     >> (see src/process.c around line 5329 and the checking against
>     >> GNUTLS_EMACS_HANDSHAKES_LIMIT) which means weʼre continually retrying
>     >> the handshake without giving the remote end a chance to send us
>     >> anything. Which I think means that our state machine for TLS
>     >> negotiation is subtly incorrect, but only on MS-Windows.
>
>     Eli> On MS-Windows, there's another state machine involved, the one
>     Eli> vis-a-vis the reader thread we start to read the stuff from the
>     Eli> network connection.  See reader_thread and sys_select in w32proc.c and
>     Eli> sys_write, sys_read, _sys_read_ahead, _sys_wait_accept, and
>     Eli> _sys_wait_connect in w32.c.
>
> Hmm, in that case I then suspect that `sys_select' is indicating that
> the socket it connected even when it isnʼt.

If that is the case, maybe we lack some flag(s) in the filedesc
structure.


[-- Attachment #1.1.2: Type: text/html, Size: 14553 bytes --]

[-- Attachment #1.2: 14D9BBA19389403AAE7B04F6D09528C2.png --]
[-- Type: image/png, Size: 32135 bytes --]

[-- Attachment #1.3: 22F9767EA55B4BA2BDE6B3027D69828E.png --]
[-- Type: image/png, Size: 86913 bytes --]

[-- Attachment #2: 45821-fix.txt --]
[-- Type: text/plain, Size: 1105 bytes --]

diff --git a/src/w32.c b/src/w32.c
index e11fa805f2..01a8616839 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -8928,7 +8928,7 @@ _sys_wait_readable (int fd)
   if (cp == NULL || cp->fd != fd || cp->status != STATUS_READ_READY)
     return STATUS_READ_ERROR;
 
-  cp->status = STATUS_READ_FAILED;
+  cp->status = STATUS_READ_IN_PROGRESS;
 
   hEv = pfn_WSACreateEvent ();
   rc = pfn_WSAEventSelect (SOCK_HANDLE (fd), hEv, FD_READ);
@@ -8944,6 +8944,8 @@ _sys_wait_readable (int fd)
       pfn_WSAEventSelect (SOCK_HANDLE (fd), NULL, 0);
       if (rc == WAIT_OBJECT_0)
         cp->status = STATUS_READ_SUCCEEDED;
+      else
+	cp->status = STATUS_READ_FAILED;
     }
   pfn_WSACloseEvent (hEv);
 
@@ -9078,7 +9080,7 @@ sys_read (int fd, char * buffer, unsigned int count)
 	    case STATUS_READ_SUCCEEDED:
 #ifdef WORKING_SELECT_EMULATION
               /* select on sockets no longer requires a 1-byte read.  */
-              if (fd_info[fd].flags & FILE_SOCKET == 0)
+              if ((fd_info[fd].flags & FILE_SOCKET) == 0)
 #endif
 		{
 		  /* consume read-ahead char */