From 2084def804f7e5bea09bb07ca8f4baf2dcf85e56 Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Fri, 3 Mar 2023 13:30:46 -0500 Subject: [PATCH 1/3] Use PIPE_NOWAIT on Windows to avoid blocking on writes --- src/io.c | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/io.c b/src/io.c index cbea654cb..faea2ceeb 100644 --- a/src/io.c +++ b/src/io.c @@ -1432,9 +1432,26 @@ _dispatch_fd_entry_create_with_fd(dispatch_fd_t fd, uintptr_t hash) #if defined(_WIN32) DWORD dwType = GetFileType((HANDLE)fd); if (dwType == FILE_TYPE_PIPE) { - unsigned long value = 1; - int result = ioctlsocket((SOCKET)fd, (long)FIONBIO, &value); - (void)dispatch_assume_zero(result); + if (_dispatch_handle_is_socket((HANDLE)fd)) { + unsigned long value = 1; + int result = ioctlsocket((SOCKET)fd, (long)FIONBIO, &value); + (void)dispatch_assume_zero(result); + } + else { + // Try to make writing nonblocking, although pipes not coming + // from Foundation.Pipe may not have FILE_WRITE_ATTRIBUTES. + DWORD pipe_mode = PIPE_NOWAIT; + if (!SetNamedPipeHandleState((HANDLE)fd, &pipe_mode, NULL, + NULL)) { + // We may end up blocking on subsequent writes, but we + // don't have a good alternative. The WriteQuotaAvailable + // from NtQueryInformationFile erroneously returns 0 when + // there is a blocking read on the other end of the pipe. + _dispatch_fd_entry_debug("failed to set PIPE_NOWAIT", + fd_entry); + } + } + _dispatch_stream_init(fd_entry, _dispatch_get_default_queue(false)); } else { @@ -2489,24 +2506,6 @@ _dispatch_operation_perform(dispatch_operation_t op) } bSuccess = TRUE; } else if (GetFileType(hFile) == FILE_TYPE_PIPE) { - // Unfortunately there isn't a good way to achieve O_NONBLOCK - // semantics when writing to a pipe. SetNamedPipeHandleState() - // can allow pipes to be switched into a "no wait" mode, but - // that doesn't work on most pipe handles because Windows - // doesn't consistently create pipes with FILE_WRITE_ATTRIBUTES - // access. The best we can do is to try to query the write quota - // and then write as much as we can. - IO_STATUS_BLOCK iosb; - FILE_PIPE_LOCAL_INFORMATION fpli; - NTSTATUS status = _dispatch_NtQueryInformationFile(hFile, &iosb, - &fpli, sizeof(fpli), FilePipeLocalInformation); - if (NT_SUCCESS(status)) { - if (fpli.WriteQuotaAvailable == 0) { - err = EAGAIN; - goto error; - } - len = MIN(len, fpli.WriteQuotaAvailable); - } OVERLAPPED ovlOverlapped = {}; bSuccess = WriteFile(hFile, buf, (DWORD)len, (LPDWORD)&processed, &ovlOverlapped); @@ -2523,6 +2522,9 @@ _dispatch_operation_perform(dispatch_operation_t op) processed = 0; } } + if (bSuccess && processed == 0) { + err = EAGAIN; + } } else { bSuccess = WriteFile(hFile, buf, (DWORD)len, (LPDWORD)&processed, NULL); From 4338bb74145d308f91b68bc06ab85514a69127b5 Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Mon, 6 Mar 2023 15:48:31 -0500 Subject: [PATCH 2/3] PR feedback and don't overwrite other pipe mode flags. --- src/io.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/io.c b/src/io.c index faea2ceeb..0bc6e0803 100644 --- a/src/io.c +++ b/src/io.c @@ -1436,19 +1436,23 @@ _dispatch_fd_entry_create_with_fd(dispatch_fd_t fd, uintptr_t hash) unsigned long value = 1; int result = ioctlsocket((SOCKET)fd, (long)FIONBIO, &value); (void)dispatch_assume_zero(result); - } - else { + } else { // Try to make writing nonblocking, although pipes not coming // from Foundation.Pipe may not have FILE_WRITE_ATTRIBUTES. - DWORD pipe_mode = PIPE_NOWAIT; - if (!SetNamedPipeHandleState((HANDLE)fd, &pipe_mode, NULL, - NULL)) { - // We may end up blocking on subsequent writes, but we - // don't have a good alternative. The WriteQuotaAvailable - // from NtQueryInformationFile erroneously returns 0 when - // there is a blocking read on the other end of the pipe. - _dispatch_fd_entry_debug("failed to set PIPE_NOWAIT", - fd_entry); + DWORD dwPipeMode = 0; + if (GetNamedPipeHandleState((HANDLE)fd, &dwPipeMode, NULL, + NULL, NULL, NULL, 0) && !(dwPipeMode & PIPE_NOWAIT)) { + dwPipeMode |= PIPE_NOWAIT; + if (!SetNamedPipeHandleState((HANDLE)fd, &dwPipeMode, + NULL, NULL)) { + // We may end up blocking on subsequent writes, but we + // don't have a good alternative. + // The WriteQuotaAvailable from NtQueryInformationFile + // erroneously returns 0 when there is a blocking read + // on the other end of the pipe. + _dispatch_fd_entry_debug("failed to set PIPE_NOWAIT", + fd_entry); + } } } From 4a73a7579d39c4439bf5dc53459f245ef53e9491 Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Tue, 7 Mar 2023 13:26:20 -0500 Subject: [PATCH 3/3] Add missing goto error; in EAGAIN case --- src/io.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/io.c b/src/io.c index 0bc6e0803..f853d9bbf 100644 --- a/src/io.c +++ b/src/io.c @@ -2528,6 +2528,7 @@ _dispatch_operation_perform(dispatch_operation_t op) } if (bSuccess && processed == 0) { err = EAGAIN; + goto error; } } else { bSuccess = WriteFile(hFile, buf, (DWORD)len,