cygwin 3.4.6 failure starting overlapped thread pipe read (code 232)

[userdocs]

Some recent updates of Cygwin have created this runtime error

failure starting overlapped thread pipe read (code 232)
mtr: Failure to start mtr-packet: Invalid argument

It worked on 3.3.5 before updating (possibly related to this? - https://cygwin.com/pipermail/cygwin-announce/2022-November/010810.html

but right now it's not working at all and i don't know why only guess it's x64 related or some other cygwin change.

[apschultz]

The named pipe issue is odd. In my environment, it is caused by ERROR_NO_DATA which should only occur if the pipe is configured as non blocking. I added a check for ERROR_NO_DATA and a small sleep to avoid excessive CPU which effectively converts the IO thread to polling mode, but it fixes the issue.

The other build issues are also pretty easy to resolve. The w32api finally includes the ICMP6 definitions which is where most of the frustration comes from.

diff --git a/packet/probe_cygwin.c b/packet/probe_cygwin.c
index f41e514..fe6bd34 100644
--- a/packet/probe_cygwin.c
+++ b/packet/probe_cygwin.c
@@ -281,7 +281,7 @@ void WINAPI on_icmp_reply(
             remote_addr6->sin6_family = AF_INET6;
             remote_addr6->sin6_port = 0;
             remote_addr6->sin6_flowinfo = 0;
-            memcpy(&remote_addr6->sin6_addr, reply6->AddressBits,
+            memcpy(&remote_addr6->sin6_addr, &reply6->Address.sin6_addr,
                    sizeof(struct in6_addr));
             remote_addr6->sin6_scope_id = 0;
         }
@@ -521,7 +521,10 @@ DWORD WINAPI icmp_service_thread(LPVOID param) {

             if (!success) {
                 err = GetLastError();
-
+                if (err == ERROR_NO_DATA) {
+                    usleep(10);
+                    continue;
+                }
                 if (err != ERROR_IO_PENDING) {
                     error_win(
                         EXIT_FAILURE, err,
diff --git a/packet/probe_cygwin.h b/packet/probe_cygwin.h
index eec001f..acaec64 100644
--- a/packet/probe_cygwin.h
+++ b/packet/probe_cygwin.h
@@ -19,32 +19,14 @@
 #ifndef PROBE_CYGWIN_H
 #define PROBE_CYGWIN_H

+#include <cygwin/in6.h>
+typedef struct in6_addr IN6_ADDR, *PIN6_ADDR, *LPIN6_ADDR;
+
 #include <arpa/inet.h>
 #include <windows.h>
 #include <iphlpapi.h>
 #include <icmpapi.h>

-/*
-    This should be in the Windows headers, but is missing from
-    Cygwin's Windows headers.
-*/
-typedef struct icmpv6_echo_reply_lh {
-    /*
-       Although Windows uses an IPV6_ADDRESS_EX here, we are using uint8_t
-       fields to avoid structure padding differences between gcc and
-       Visual C++.  (gcc wants to align the flow info to a 4 byte boundary,
-       and Windows uses it unaligned.)
-     */
-    uint8_t PortBits[2];
-    uint8_t FlowInfoBits[4];
-    uint8_t AddressBits[16];
-    uint8_t ScopeIdBits[4];
-
-    ULONG Status;
-    unsigned int RoundTripTime;
-} ICMPV6_ECHO_REPLY,
-*PICMPV6_ECHO_REPLY;
-
 /*
        Windows requires an echo reply structure for each in-flight
        ICMP probe.

[apschultz]

Some googling of w32 APIs suggest the methodology used for cygwin is improper to start. The probe_cygwin code is using anonymous pipes which don't seem to support the OVERLAPPED methods:

https://learn.microsoft.com/en-us/windows/win32/ipc/anonymous-pipe-operations

[matt-kimball]

Some googling of w32 APIs suggest the methodology used for cygwin is improper to start. The probe_cygwin code is using anonymous pipes which don't seem to support the OVERLAPPED methods:

That's wild. Do you have a suggestion for an alternative?

[apschultz]

The pipes are being used as queues between threads and are a completely unnecessary construct. A simple mutex protected linked list and a pthread_cond_t object to wake the reader (or a w32 Event object if you want to keep with that API) would suffice. However, if there is a desire to limit the amount of change in code, there are examples of using named pipes to mimic anonymous pipes which would allow the asynchronous behaviors being used.

https://github.com/jeremybernstein/shell/blob/master/PipeEx.c is an example based on Microsoft Example code which would probably suit the use case.

[rewolff]

Although I haven't used windows on any of my machines for decades, I would prefer the code to be similar for different OSes. So windows uses a pipe between mtr-packet and mtr-gui, thus then I would prefer it that on windows/cygwin a similar mechanism would be used as opposed to something that might be preferred if there was no non-windows implementation.

[matt-kimball]

Some googling of w32 APIs suggest the methodology used for cygwin is improper to start. The probe_cygwin code is using anonymous pipes which don't seem to support the OVERLAPPED methods:

https://learn.microsoft.com/en-us/windows/win32/ipc/anonymous-pipe-operations

It seems that the Cygwin implementation of pipe() actually creates the pipe as a Windows named pipe (as opposed to as an anonymous pipe). I think the overlapped read is fine. See the implementation of fhandler_pipe::create:

https://www.cygwin.com/cgit/newlib-cygwin/tree/winsup/cygwin/fhandler/pipe.cc#n737

This does bring us back to the problem that the overlapped read fails to start. I believe this commit to Cygwin, which always puts the read end of pipes in non-blocking mode due to some issue with signal delivery, may have caused this to break in mtr:

https://www.cygwin.com/cgit/newlib-cygwin/commit/?id=9e4d308cd592fe383dec58ea6523c1b436888ef8