Skip to content

tool: Implement non-blocking STDIN read on Windows#17572

Closed
denandz wants to merge 23 commits intocurl:masterfrom
denandz:windows-stdin-handling
Closed

tool: Implement non-blocking STDIN read on Windows#17572
denandz wants to merge 23 commits intocurl:masterfrom
denandz:windows-stdin-handling

Conversation

@denandz
Copy link
Contributor

@denandz denandz commented Jun 10, 2025

Implements a seperate read thread for STDIN on Windows when curl is run with -T/--upload-file .

This uses a similar technique to the nmap/ncat project, spawning a seperate thread which creates a loop-back bound socket, sending STDIN into this socket, and reading from the other end of said TCP socket in a non-blocking way in the rest of curl.

Fixes #17451

Implements a seperate read thread for STDIN on Windows when
curl is run with -T/--upload-file .

This uses a similar technique to the nmap/ncat project, spawning
a seperate thread which creates a loop-back bound socket, sending
STDIN into this socket, and reading from the other end of said
TCP socket in a non-blocking way in the rest of curl.

Closes curl#17451
@denandz denandz marked this pull request as draft June 10, 2025 11:19
@testclutch

This comment was marked as outdated.

@denandz
Copy link
Contributor Author

denandz commented Jun 13, 2025

Alright! This seems to be building / testing successfully now. Looks like the two fails on macOS are due to test suites timing out if I'm reading the log correctly. Not sure if that's related to my changes?

I think this PR is ready for review,.

@denandz denandz marked this pull request as ready for review June 13, 2025 12:03
@denandz denandz requested a review from vszakats June 13, 2025 12:43
@icing icing requested a review from jay June 13, 2025 12:47
@jay
Copy link
Member

jay commented Jun 15, 2025

Does #17569 work for you?

@denandz
Copy link
Contributor Author

denandz commented Jun 15, 2025

Does #17569 work for you?

Hi Jay. That pull request is for a different issue (performance of -T .). This pull request is to make -T . work on Windows (see issue #17451 ). That pull request does not implement non-blocking stdin support under Windows.

To answer your question though - Yes, I see improved performance under linux with #17569 and even further improved performance with #17566 - but that's not what this PR is about. Stdin performance issue is #17553

Copy link
Member

@vszakats vszakats left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@vszakats vszakats changed the title curl: Implement non-blocking STDIN read on Windows tool: Implement non-blocking STDIN read on Windows Jun 15, 2025
@denandz
Copy link
Contributor Author

denandz commented Jun 16, 2025

Thanks team! Let me know if there is anything else you need from my end to get this merged

@bagder bagder closed this in 9a26633 Jun 21, 2025
@bagder
Copy link
Member

bagder commented Jun 21, 2025

Thanks!

denandz added a commit to denandz/curl that referenced this pull request Jun 21, 2025
Implements a seperate read thread for STDIN on Windows when curl is run
with -T/--upload-file .

This uses a similar technique to the nmap/ncat project, spawning a
seperate thread which creates a loop-back bound socket, sending STDIN
into this socket, and reading from the other end of said TCP socket in a
non-blocking way in the rest of curl.

Fixes curl#17451
Closes curl#17572
vszakats added a commit that referenced this pull request Sep 19, 2025
Replace `_beginthreadex()` C runtime calls with native win32 API
`CreateThread()`. The latter was already used in `src/tool_doswin.c`
and in UWP and Windows CE builds before this patch. After this patch
all Windows flavors use it. To drop PP logic and simplify code.

While working on this it turned out that `src/tool_doswin.c` calls
`TerminateThread()`, which isn't recommended by the documentation,
except for "the most extreme cases". This patch makes no attempt
to change that code.
Ref: 9a26633 #17572
Ref: https://learn.microsoft.com/windows/win32/api/processthreadsapi/nf-processthreadsapi-terminatethread

Also:
- use `WaitForSingleObjectEx()` on all desktop Windows.
  Ref: 4be80d5
  Ref: https://sourceforge.net/p/curl/feature-requests/82/
  Ref: https://learn.microsoft.com/windows/win32/api/synchapi/nf-synchapi-waitforsingleobjectex
- tests: drop redundant casts.
- lib3207: fix to not rely on thread macros when building without thread
  support.

Assisted-by: Jay Satiro
Assisted-by: Marcel Raad
Assisted-by: Michał Petryka
Follow-up to 3802910 #11625

Closes #18451
vszakats added a commit that referenced this pull request Sep 22, 2025
Replace `WSASocketW()` with `CURL_SOCKET()`. Also replace a call
to `socketclose()` with `sclose()`. According to a comment,
`socketclose()` was chosen to silence test 1498 (and 2300) reporting
`MEMORY FAILURE`. These reports were accurate, and were caused by
calling `WSASocketW()` instead of `socket()` (now `CURL_SOCKET()`).

This also fixes the curl `sclose()` call on an error branch, which is
now correctly paired with a curl socket open. The mismatched open/close
calls caused an issue in TrackMemory-enabled (aka `CURLDEBUG`) builds.

Docs confirm that `socket()` is defaulting to overlapped I/O, matching
the replaced `WSASocketW()` call:
https://learn.microsoft.com/windows/win32/api/winsock2/nf-winsock2-socket#remarks

Also:
- checksrc: ban `WSASocket*()` functions.
- report `SOCKERRNO` instead of `GetLastError()` for socket calls,
  to match the rest of the codebase.

Follow-up to 9a26633 #17572

Closes #18633
vszakats added a commit to vszakats/curl that referenced this pull request Jan 7, 2026
- fix duplicate space in error message (UWP)

Follow-up to 9a26633 curl#17572
vszakats added a commit that referenced this pull request Jan 9, 2026
Prior to this patch, some Windows logic, including a Windows-specific
warning message was compiled in for all platforms.

Also:
- fix double space in warning message on UWP.
- formatting.

Follow-up to 9a26633 #17572

Closes #20213
vszakats added a commit to vszakats/curl that referenced this pull request Jan 27, 2026
vszakats added a commit that referenced this pull request Jan 27, 2026
For general readability. Also to match the rest of the source code.

- `SOCKADDR` -> `struct sockaddr`
- `SOCKADDR_IN` -> `struct sockaddr_in`
- `== SOCKET_ERROR` -> `== -1` or silent `!= 0`

Follow-up to 9a26633 #17572

Closes #20452
vszakats added a commit to vszakats/curl that referenced this pull request Jan 27, 2026
vszakats added a commit to vszakats/curl that referenced this pull request Jan 28, 2026
vszakats added a commit that referenced this pull request Jan 28, 2026
For general readability. Also to match the rest of the source code.

- bump `send()` result type from `int` to `ssize_t`.
- fix an `int` to be `curl_socklen_t`.
- `.S_un.S_addr` -> `.s_addr`.
- `SD_RECEIVE` -> `SHUT_RD`.
- `SD_SEND` -> `SHUT_WR`.

Follow-up to a81ab3e #20452
Follow-up to 9a26633 #17572

Closes #20457
vszakats added a commit to vszakats/curl that referenced this pull request Jan 28, 2026
vszakats added a commit to vszakats/curl that referenced this pull request Jan 28, 2026
vszakats added a commit that referenced this pull request Jan 28, 2026
Also:
- checksrc: ban `send` and `recv`, as documented in `CODE_STYLE.md`.

Follow-up to 9a26633 #17572
Ref: a585cc3 #20097
Ref: #20441

Closes #20459
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

-T . / --upload-file . non-blocking STDIN/STDOUT does not work on Windows

5 participants