win,pipe: restoring fallback handling for blocking pipes#4511
Merged
vtjnash merged 2 commits intolibuv:v1.xfrom Aug 22, 2024
Merged
win,pipe: restoring fallback handling for blocking pipes#4511vtjnash merged 2 commits intolibuv:v1.xfrom
vtjnash merged 2 commits intolibuv:v1.xfrom
Conversation
In libuv#4470, I accidentally copied the bug from unix, where calling uv_stream_set_blocking can cause the whole process to hang on a read. However, unlike unix, where libuv attempts to set the O_NONBLOCK flag in uv_pipe_open (as long as the handle never gets passed to uv_spawn), the NT kernel is not capable of enabling OVERLAPPED operation later (but fortunately, it also cannot disable it later too). This implementation might be good to copy to unix (using FIONREAD) to address the same bug that happens there if the user has called uv_spawn or uv_stream_set_non_blocking on this handle in the past.
Closed
saghul
approved these changes
Aug 20, 2024
vtjnash
commented
Aug 20, 2024
santigimeno
approved these changes
Aug 22, 2024
Contributor
|
(Unfortunately, it looks like the Neovim tests still hang even with this fix, albeit in a different place.... https://github.com/neovim/neovim/actions/runs/10512280463/job/29126681847?pr=29915) |
Member
Author
|
Thanks. Looks like I missed an assignment to |
vtjnash
added a commit
that referenced
this pull request
Sep 5, 2024
Yet another followup to #4511. The functional/legacy/increment_spec.lua test failed most of the time without this, and passes consistently with it. It seemed unexpected this code path gets reached (perhaps imply that the user wrote zero bytes?), but good to fix of course.
vtjnash
added a commit
to vtjnash/libuv
that referenced
this pull request
Sep 30, 2024
The implementation of IPC pipe in libuv on Windows does not properly support async reading. This means we cannot set the more parameter without likely causing hangs. Sorry this is yet another followup to libuv#4511. Fixes libuv#4548
santigimeno
pushed a commit
that referenced
this pull request
Oct 3, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In #4470, I accidentally copied the bug from unix, where calling uv_stream_set_blocking can cause the whole process to hang on a read. However, unlike unix, where libuv attempts to set the O_NONBLOCK flag in uv_pipe_open (as long as the handle never gets passed to uv_spawn), the NT kernel is not capable of enabling OVERLAPPED operation later (but fortunately, it also cannot disable it later too).
This implementation might be good to copy to unix (using FIONREAD) to address the same bug that happens there if the user has called uv_spawn or uv_stream_set_non_blocking on this handle in the past.