Package: emacs;
Reported by: Lars Ingebrigtsen <larsi <at> gnus.org>
Date: Tue, 12 Jan 2021 18:09:02 UTC
Severity: wishlist
Tags: patch
Merged with 9586
Found in version 28.0.50
View this message in rfc822 format
From: Robert Pluim <rpluim <at> gmail.com> To: Alex Matei <matei.alexandru <at> live.com> Cc: Eli Zaretskii <eliz <at> gnu.org>, "45821 <at> debbugs.gnu.org" <45821 <at> debbugs.gnu.org> Subject: bug#45821: Emacs UDP support on Windows Date: Tue, 03 Jan 2023 09:51:11 +0100
[Message part 1 (text/plain, inline)]
>>>>> On Mon, 2 Jan 2023 22:56:04 +0000, Alex Matei <matei.alexandru <at> live.com> said:
Alex> Thanks for the tips with logging!
Alex> I think I found the fix (at least for one of the issues?)
Alex> Please see the attached patch/diff for the fix.
Alex> The code will always ignore (aka ‘eat’) one character if the file
Alex> descriptor had the least significant bit set (0x1, so this would mean
Alex> for any file descriptor with READ flag set)
Alex> [cid:image004.png <at> 01D91EBA.5498F140]
Hmm, my local version already had that fixed, so I guess I posted the
wrong patch to the bug. But I still think you found a fix (see below)
Alex> Based on operator precedence rules, FILE_SOCKET == 0 will be evaluated
Alex> first (before &) , resulting in 0, which would effectively cause the
Alex> condition to always be false
Alex> This behavior can be observed when executing a call to
Alex> ‘(async-shell-command)’ (-> file descriptor flags 0x191) on a build
Alex> with the patch vs one without the patch applied (see below, attached
Alex> image)
Alex> * My suspicion is that this behavior is similar to what TLS was
Alex> experiencing but just easier to reproduce (note: there might other
Alex> issues with TLS , but so far, after applying this fix I have not run
Alex> into any other issues)
Alex> [cid:image005.png <at> 01D91EBA.5498F140]
Alex> @Robert Pluim<mailto:rpluim <at> gmail.com> I don’t know if this was the
Alex> only missing piece, but from all my tests I couldn’t see any issues
Alex> with TLS anymore, and with navigating https://gnu.org in EWW..
Alex> I wanted to thank you for creating this patch 🙏, and giving me pointers on how to apply and debug it.
Iʼd run the networking portion of our test suite to see if everything
works as intended (you might need to install the GnuTLS cli tools). eg:
cd test
make network-stream-tests
Alex> p.s.
Alex> - The patch also adds STATUS_READ_IN_PROGRESS state for the new
Alex> ‘_sys_wait_readable’ function , based on what the read ahead logic was
Alex> doing already (idk if it is needed, but it is a nice mirror, and a
Alex> good status code to reflect, although I am not convinced anyone cares
Alex> about this state transition..)
I think this is actually the fix, since I added it locally and TLS
started working, but I donʼt understand the code enough to be definite
about that. More testing required 😀
Can you post the full patch just to ensure weʼre all talking about the
same changes? Iʼve attached what Iʼm working from
Robert
--
[0001-Make-UDP-sockets-work-on-Windows.patch (text/x-diff, inline)]
From 2ae91ed495f3972ddac383bd5f63c47946d5cdb5 Mon Sep 17 00:00:00 2001
From: Robert Pluim <rpluim <at> gmail.com>
Date: Mon, 27 Sep 2021 17:25:27 +0200
Subject: [PATCH] Make UDP sockets work on Windows
To: emacs-devel <at> gnu.org
* admin/CPP-DEFINES: remove BROKEN_DATAGRAM_SOCKETS
* nt/inc/ms-w32.h: remove BROKEN_DATAGRAM_SOCKETS.
* src/process.c (DATAGRAM_SOCKETS): remove dependency on
!BROKEN_DATAGRAM_SOCKETS.
* src/w32.c (_sys_wait_readable): New function. Calls
pfn_WSAEventSelect for sockets to see if data can be read, so the
1-byte readahead is no longer necessary.
(sys_read): Only do 1-byte readahead for non-sockets.
* src/w32.h: Add prototype for _sys_wait_readable.
* src/w32proc.c (reader_thread): Call _sys_wait_readable for socket
handles.
---
admin/CPP-DEFINES | 1 -
nt/inc/ms-w32.h | 4 ----
src/process.c | 8 +++-----
src/w32.c | 49 +++++++++++++++++++++++++++++++++++++++++++----
src/w32.h | 1 +
src/w32proc.c | 2 ++
6 files changed, 51 insertions(+), 14 deletions(-)
diff --git a/admin/CPP-DEFINES b/admin/CPP-DEFINES
index 06986ec8f48..2704bc57675 100644
--- a/admin/CPP-DEFINES
+++ b/admin/CPP-DEFINES
@@ -103,7 +103,6 @@ getting at the full user name. Only MSDOS overrides the default.
anymore, so they can be removed.
AMPERSAND_FULL_NAME
-BROKEN_DATAGRAM_SOCKETS
BROKEN_GET_CURRENT_DIR_NAME
BROKEN_PTY_READ_AFTER_EAGAIN
DEFAULT_SOUND_DEVICE
diff --git a/nt/inc/ms-w32.h b/nt/inc/ms-w32.h
index 58be1199345..535d24cd57b 100644
--- a/nt/inc/ms-w32.h
+++ b/nt/inc/ms-w32.h
@@ -89,10 +89,6 @@ #define _CALLBACK_ __cdecl
Look in <sys/time.h> for a timeval structure. */
#define HAVE_TIMEVAL 1
-/* Our select emulation does 1-byte read-ahead waiting for received
- packets, so datagrams are broken. */
-#define BROKEN_DATAGRAM_SOCKETS 1
-
#define MAIL_USE_SYSTEM_LOCK 1
/* Define to 1 if GCC-style __attribute__ ((__aligned__ (expr))) works. */
diff --git a/src/process.c b/src/process.c
index 67d1d3e425f..8f7fccfe538 100644
--- a/src/process.c
+++ b/src/process.c
@@ -234,11 +234,9 @@ #define PIPECONN1_P(p) (EQ (p->type, Qpipe))
"non-destructive" select. So we require either native select,
or emulation of select using FIONREAD. */
-#ifndef BROKEN_DATAGRAM_SOCKETS
-# if defined HAVE_SELECT || defined USABLE_FIONREAD
-# if defined HAVE_SENDTO && defined HAVE_RECVFROM && defined EMSGSIZE
-# define DATAGRAM_SOCKETS
-# endif
+#if defined HAVE_SELECT || defined USABLE_FIONREAD
+# if defined HAVE_SENDTO && defined HAVE_RECVFROM && defined EMSGSIZE
+# define DATAGRAM_SOCKETS
# endif
#endif
diff --git a/src/w32.c b/src/w32.c
index 47d79abc5b0..ef7d3de861b 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -8940,6 +8940,43 @@ _sys_wait_accept (int fd)
return cp->status;
}
+int
+_sys_wait_readable (int fd)
+{
+ HANDLE hEv;
+ child_process * cp;
+ int rc;
+
+ if (fd < 0 || fd >= MAXDESC)
+ return STATUS_READ_ERROR;
+
+ cp = fd_info[fd].cp;
+
+ if (cp == NULL || cp->fd != fd || cp->status != STATUS_READ_READY)
+ return STATUS_READ_ERROR;
+
+ cp->status = STATUS_READ_FAILED;
+
+ hEv = pfn_WSACreateEvent ();
+ rc = pfn_WSAEventSelect (SOCK_HANDLE (fd), hEv, FD_READ);
+ if (rc != SOCKET_ERROR)
+ {
+ do
+ {
+ rc = WaitForSingleObject (hEv, 500);
+ Sleep (5);
+ } while (rc == WAIT_TIMEOUT
+ && cp->status != STATUS_READ_ERROR
+ && cp->char_avail);
+ pfn_WSAEventSelect (SOCK_HANDLE (fd), NULL, 0);
+ if (rc == WAIT_OBJECT_0)
+ cp->status = STATUS_READ_SUCCEEDED;
+ }
+ pfn_WSACloseEvent (hEv);
+
+ return cp->status;
+}
+
int
_sys_wait_connect (int fd)
{
@@ -9065,10 +9102,14 @@ sys_read (int fd, char * buffer, unsigned int count)
return -1;
case STATUS_READ_SUCCEEDED:
- /* consume read-ahead char */
- *buffer++ = cp->chr;
- count--;
- nchars++;
+ /* select on sockets no longer requires a 1-byte read. */
+ if ((fd_info[fd].flags & FILE_SOCKET) == 0)
+ {
+ /* consume read-ahead char */
+ *buffer++ = cp->chr;
+ count--;
+ nchars++;
+ }
cp->status = STATUS_READ_ACKNOWLEDGED;
ResetEvent (cp->char_avail);
diff --git a/src/w32.h b/src/w32.h
index a3d0b75359a..4eba0fabe94 100644
--- a/src/w32.h
+++ b/src/w32.h
@@ -177,6 +177,7 @@ #define FILE_DONT_CLOSE 0x1000
extern int _sys_read_ahead (int fd);
extern int _sys_wait_accept (int fd);
+extern int _sys_wait_readable (int fd);
extern int _sys_wait_connect (int fd);
extern HMODULE w32_delayed_load (Lisp_Object);
diff --git a/src/w32proc.c b/src/w32proc.c
index 77a4ac1ff7e..19d9fc7d0e7 100644
--- a/src/w32proc.c
+++ b/src/w32proc.c
@@ -1243,6 +1243,8 @@ reader_thread (void *arg)
rc = _sys_wait_connect (fd);
else if (fd >= 0 && (fd_info[fd].flags & FILE_LISTEN) != 0)
rc = _sys_wait_accept (fd);
+ else if (fd_info[fd].flags & FILE_SOCKET)
+ rc = _sys_wait_readable (fd);
else
rc = _sys_read_ahead (fd);
--
2.38.1.420.g319605f8f0
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.