Fix SSH hang (next attempt) #103
Merged
Conversation
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
This reverts the fix in preparation for integrating a newer iteration of the fix. Signed-off-by: Johannes Schindelin <[email protected]>
If ssh is used with non-cygwin pipe reader, ssh some times hangs. This happens when non-cygwin git (Git for Windows) starts cygwin ssh. The background of the bug is as follows. Before attempting to NtWriteFile() in raw_write() in non-blocking mode, the amount of writable space in the pipe is checked by calling NtQueryInformationFile with FilePipeLocalInformation parameter. The same is also done by pipe_data_available() in select.cc. However, if the read side of the pipe is simultaneously consuming data, NtQueryInformationFile() returns less value than the amount of writable space, i.e. the amount of writable space minus the size of buffer to be read. This does not happen when the reader is a cygwin app because cygwin read() for the pipe attempts to read the amount of the data in the pipe at most. This means NtReadFile() never enters a pending state. However, if the reader is non-cygwin app, this cannot be expected. As a workaround for this problem, the code checking the pipe space temporarily attempts to toggle the pipe-mode. If the pipe contains data, this operation fails with STATUS_PIPE_BUSY indicating that the pipe is not empty. If it succeeds, the pipe is considered empty. The current code uses this technic only when NtQueryInformationFile() retuns zero. Therefore, if NtQueryInformationFile() returns 1, the amount of writable space is assumed to be 1 even in the case that e.g. the pipe size is 8192 bytes and reader is pending to read 8191 bytes. Even worse, the current code fails to write more than 1 byte to 1 byte pipe space due to the remnant of the past design. Then the reader waits for data with 8191 bytes buffer while the writer continues to fail to write to 1 byte space of the pipe. This is the cause of the deadlock. In practice, when using Git for Windows in combination with Cygwin SSH, it has been observed that a read of 8191 bytes is occasionally issued against a pipe with 8192 bytes of available space. With this patch, the blocking-mode-toggling-check is performed even if NtQueryInformationFile() returns non-zero value so that the amount of the writable space in the pipe is always estimated correctly. Addresses: git-for-windows/git#5682 Fixes: 7ed9adb ("Cygwin: pipe: Switch pipe mode to blocking mode by default") Reported-by: Vincent-Liem (@github), Johannes Schindelin <[email protected]> Reviewed-by: Signed-off-by: Takashi Yano <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
The commit "Cygwin: pipe: Fix SSH hang with non-cygwin pipe reader" modifies how the amount of writable data is evaluated. This patch updates the source comments to align with that change. Reviewed-by: Signed-off-by: Takashi Yano <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
... rather than 1 when the pipe space estimation fails, so that select() and raw_wrie() can perform appropriate fallback handling. In select(), even if pipe space is unknown, return writable to avoid deadlock. Even with select() returns writable, write() can blocked anyway, if data is larger than pipe space. In raw_write(), if the pipe is real non-blocking mode, attempting to write larger data than pipe space is safe. Otherwise, return error. Reviewed-by: Signed-off-by: Takashi Yano <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
Member
Author
|
/open pr The workflow run was started |
1 task
github-actions bot
pushed a commit
to git-for-windows/build-extra
that referenced
this pull request
Jun 30, 2025
Cloning large repositories via SSH [frequently hung with Git for Windows v2.50.0](git-for-windows/git#5688), which [was fixed](git-for-windows/msys2-runtime#103). Signed-off-by: gitforwindowshelper[bot] <[email protected]>
This was referenced Jul 1, 2025
MarkEWaite
added a commit
to MarkEWaite/docker-lfs
that referenced
this pull request
Jul 2, 2025
* Cloning: large repositories via SSH frequently hung with Git for Windows v2.50.0 git-for-windows/git#5688 fixed in git-for-windows/msys2-runtime#103 * In: Git for Windows v2.50.0, operations using the POSIX emulation layer (cloning via SSH, generating the Bash prompt) cannot be interrupted by Ctrl+C, fixed in git-for-windows/msys2-runtime#104 * Git: for Windows v2.50.0 is unable to initialize Git repositories on Windows Server 2016 git-for-windows/git#5695), fixed in git-for-windows/git#5700
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
This first reverts my previous attempt and then applies the patches proposed on the Cygwin-patches mailing list (#1, #2, #3).
This fixes git-for-windows/git#5682 and git-for-windows/git#5688.