tool_doswin: don't use TerminateThread in stdin relay

[11soda11]

It is a dangerous function. Remove usage by only starting the thread after setup completed successfully.

Ref: https://learn.microsoft.com/windows/win32/api/processthreadsapi/nf-processthreadsapi-terminatethread
Ref: #21467 (comment)
Ref: #18996 (comment)
Ref: #18451
Follow-up to 9a26633 #17572

[11soda11]

I added a test because i noticed there wasn't one for async stdin, only for normal stdin upload.

the failing test seems to be unrelated

[11soda11]

both tests are still failing in the torture test because of the socket fd which the main thread passes to the background thread. I don't see a solution other than making valgrind ignore it somehow because the socket needs to be alive throughout the whole program lifecycle.

[vszakats]

both tests are still failing in the torture test because of the socket fd which the main thread passes to the background thread. I don't see a solution other than making valgrind ignore it somehow because the socket needs to be alive throughout the whole program lifecycle.

Is there perhaps a way to wait for that thread before exit,
and/or just close handles? or do this via atexit()/signal_handler()?

[11soda11]

Is there perhaps a way to wait for that thread before exit, and/or just close handles? or do this via atexit()/signal_handler()?

waiting for the thread is not really an option because it is inside a blocking read call most of the time and the fd needs to be open until exit. atexit seems to work tho

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[11soda11]

I can no longer reproduce either problem with your updated branch. The no content problem involved a debug break which was interactive, as I showed earlier. My guess is it was a timing dependent problem and had nothing to do with the debugger.

I don't really have a debugger setup on windows, so I tried to test it with curl_wait_ms() to simulate a delay with no luck.

But I guess it is fixed now if you can't reproduce it either.

[jay]

Viktor before this is landed I'd like more time to hear from @denandz to make sure everything is ok with his setup which may be more complicated than our tests.

[vszakats]

Viktor before this is landed I'd like more time to hear from @denandz to make sure everything is ok with his setup which may be more complicated than our tests.

Sounds good to me. Short of his blessing, I'd be happy to land this on the next cycle, if no one else is reporting issues.

Do we know specifics about @denandz setup?

[jay]

Do we know specifics about @denandz setup?

Yes:

Ill have some time this week to do some testing as well. There's a more detailed use-case in https://github.com/denandz/httpstream2tcp

[denandz]

Thank you for your patience, team. Busy week on my end.

I've successfully tested these changes with httpstream2tcp and OpenSSH. No issues on my end, LGTM and blessings given! 🙏

[11soda11]

sry for the noise. I had committed a faulty change, which I now reverted by resetting to the previous commit, so nothing has changed in total.

However, there is a flaky netbsd test which is failing now.

I don't have anything more to add to this pr, its done from my side.

[jay]

I've squashed your commits and rebased on upstream/master to prepare for landing this. I've force-pushed the proposed final commit to your branch. Since there was dual author information I used the one with your real name (also the name on your github profile). Please amend the commit meta if necessary. After it passes CI I will take a final look.

for example git commit --amend --author="YOU MUST <CHANGE@THIS>"

While reviewing this PR I found two issues related to this stdin upload area but separate from this PR so I will address them separately. The first is how curl can ignore truncated data on stdin (this is how I saw 0 content being uploaded in the debugger), filed as #22682. The second is CPU usage issue like if stdin is used like to type to peer and back and forth and it's waiting a lot, filed as #22640.

[11soda11]

Please amend the commit meta if necessary

done. Thank you for your work

[jay]

Thanks