fix(win): prevent UI hang — spawn git without leaking handles into the child#799
Open
Flipper1994 wants to merge 1 commit into
Open
fix(win): prevent UI hang — spawn git without leaking handles into the child#799Flipper1994 wants to merge 1 commit into
Flipper1994 wants to merge 1 commit into
Conversation
On Windows with the UI enabled (--ui=true), list_projects hangs forever and wedges the single-threaded HTTP server, so the web UI never loads. Root cause: list_projects resolves git context per project via git_capture -> cbm_popen (git_context.c). On Windows cbm_popen was _popen, which spawns the child with CreateProcess(bInheritHandles=TRUE) and leaks every inheritable handle into the git child -- including the Winsock/AFD handles that exist only in UI mode. git-for-Windows (MSYS2/Cygwin) classifies each inherited handle via NtQueryObject on startup, which deadlocks on an inherited socket/AFD handle, so git never runs and the server blocks forever in fgets on git's stdout pipe. Confirmed with gdb (request thread in fgets/git_capture, git.exe child in ntdll!ZwQueryObject). The plain MCP-stdio server and the CLI are unaffected -- no socket handles to inherit -- which is why only the UI hangs. Fix: reimplement cbm_popen on Windows with CreateProcess + STARTUPINFOEX and an explicit PROC_THREAD_ATTRIBUTE_HANDLE_LIST containing only the stdout write-end and a NUL handle for stdin/stderr. The git child now inherits only the pipe, so there is no foreign handle for NtQueryObject to deadlock on. cbm_pclose reaps via a small FILE*->HANDLE table. The POSIX path is unchanged (popen already sets O_CLOEXEC). Centralized in cbm_popen, so it also covers watcher.c and the git-history pass. Signed-off-by: Flipper <jacobphilipp@ymail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
|
Thanks for the Windows UI hang fix for #798. Triage: high-priority Windows/UI stability. Review will focus on the custom Windows |
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
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.
What does this PR do?
Fixes a Windows-only deadlock that makes the web UI hang forever. With
--ui=true,the MCP tool
list_projectsnever returns and, because the UI HTTP server issingle-threaded, the whole server stops responding — the UI page loads but stays
blank. Closes #798.
Root cause
list_projectsshells out to git per project for git context(
git_capture→cbm_popen,src/git/git_context.c). On Windowscbm_popenwasthe CRT
_popen, which doesCreateProcess(bInheritHandles = TRUE)and leaksevery inheritable handle into the git child — including the Winsock/AFD handles
that exist only in UI mode. git-for-Windows (MSYS2/Cygwin) classifies each inherited
handle via
NtQueryObjecton startup, which deadlocks on an inheritedsocket/AFD handle. git never runs, so the server blocks forever in
fgetson git'sstdout pipe. Confirmed with gdb: request thread in
fgets/git_capture, thegit.exechild inntdll!ZwQueryObject.The plain MCP-stdio server and the CLI are unaffected (no socket handles to inherit),
which is why only the UI hangs.
The fix
Reimplement
cbm_popenon Windows (src/foundation/compat_fs.c) to spawn viaCreateProcess+STARTUPINFOEXwith an explicitPROC_THREAD_ATTRIBUTE_HANDLE_LISTcontaining only the stdout write-end and aNULhandle for the child's stdin/stderr. The git child now inherits only the pipe —
no sockets, no MCP stdin pipe, no Winsock handles — so there is no foreign handle for
NtQueryObjectto deadlock on.cbm_pclosereaps the process via a smallFILE*→HANDLEtable. The POSIX path is unchanged (popenalready opens its pipeO_CLOEXEC). This is centralized incbm_popen, so it also covers the watcher(
src/watcher/watcher.c) and git-history pass, which shell out to git the same way.Validation (Windows, production
cbm-with-uibinary)POST /rpc list_projects(UI)git/cmdprocessesChecklist
git commit -s)test.shbuilds with ASan/UBSan, whichare unavailable on the MSYS2/MinGW toolchain used to reproduce this; the fix is
#ifdef _WIN32-only and does not touch the POSIX path.git-for-Windows) and does not reproduce under the Linux CI; happy to add a
Windows-only test under
tests/windows/if preferred.