Skip to content

Commit

Permalink
Update Cygwin ICMP service thread for asynchronous pipes
Browse files Browse the repository at this point in the history
Recent versions of Cygwin implement pipe() using Windows' named
pipes, and put the read end of the pipe in FILE_PIPE_COMPLETE_OPERATION
mode, which doesn't allow overlapped I/O operations.

For the relevant commit in the Cygwin repository, see
9e4d308cd592fe383dec58ea6523c1b436888ef8

Happily, this mode is approximately equivalent to Unix's non-blocking
read mode, so we can avoid overlapped I/O entirely, and instead wait
on the Windows handle for the read end of the pipe when we need an
alertable wait to receive ICMP probe completions.

Thanks to Adam Schultz for research into this issue and a first
attempt at a fix.
  • Loading branch information
matt-kimball committed Oct 23, 2023
1 parent adfa754 commit bd0f147
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 86 deletions.
79 changes: 14 additions & 65 deletions packet/probe_cygwin.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -479,65 +479,16 @@ DWORD WINAPI icmp_service_thread(LPVOID param) {
struct net_state_t *net_state;
struct icmp_thread_request_t *request;
DWORD wait_status;
OVERLAPPED overlapped;
HANDLE event;
BOOL success;
bool read_pending;
DWORD read_count;
int err;

/*
We need an event to signal completion of reads from the request
pipe.
*/
event = CreateEvent(NULL, TRUE, FALSE, NULL);
if (event == NULL) {
error_win(
EXIT_FAILURE, GetLastError(),
"failure creating ICMP thread event");
}
int read_count;

net_state = (struct net_state_t *)param;
read_pending = false;
while (true) {
/*
Start a new read on the request pipe if none is
currently pending.
*/
if (!read_pending) {
request = NULL;

ResetEvent(event);

memset(&overlapped, 0, sizeof(OVERLAPPED));
overlapped.hEvent = event;

success = ReadFile(
net_state->platform.thread_in_pipe_read_handle,
&request,
sizeof(struct icmp_thread_request_t *),
NULL,
&overlapped);

if (!success) {
err = GetLastError();

if (err != ERROR_IO_PENDING) {
error_win(
EXIT_FAILURE, err,
"failure starting overlapped thread pipe read");
}
}

read_pending = true;
}

/*
Wait for either the request read to complete, or
Wait for either the pipe to be readable or for
an APC which completes an ICMP probe.
*/
wait_status = WaitForSingleObjectEx(
event,
net_state->platform.thread_in_pipe_read_handle,
INFINITE,
TRUE);

Expand All @@ -546,18 +497,16 @@ DWORD WINAPI icmp_service_thread(LPVOID param) {
the request from the pipe.
*/
if (wait_status == WAIT_OBJECT_0) {
read_pending = false;

success = GetOverlappedResult(
net_state->platform.thread_in_pipe_read_handle,
&overlapped,
&read_count,
FALSE);

if (!success) {
error_win(
EXIT_FAILURE, GetLastError(),
"failure completing overlapped thread pipe read");
read_count = read(
net_state->platform.thread_in_pipe_read,
&request,
sizeof(struct icmp_thread_request_t *));

if (read_count == -1) {
error(
EXIT_FAILURE,
errno,
"failure reading probe request pipe");
}

if (read_count == 0) {
Expand Down
24 changes: 3 additions & 21 deletions packet/probe_cygwin.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit bd0f147

Please sign in to comment.