Prevent stale index.lock files from diffs rendered through a pty on Windows - #5888
Open
stefanhaller wants to merge 3 commits into
Open
Prevent stale index.lock files from diffs rendered through a pty on Windows#5888stefanhaller wants to merge 3 commits into
stefanhaller wants to merge 3 commits into
Conversation
…indows At the end of a diff against the worktree, git re-reads and refreshes the index and writes it back if it found stale stat information (diff.autoRefreshIndex, on by default). It holds index.lock for the whole refresh; GIT_OPTIONAL_LOCKS does not cover this lock, and the window scales with the size of the repository (~150ms for a 6k-file repository with a warm stat cache). On Windows, a pty task that is stopped because the user moved on terminates its git process at an arbitrary point: tearing down the pseudoconsole delivers CTRL_CLOSE_EVENT, which git leaves to the default handler, which simply calls ExitProcess. If that lands inside the refresh, a stale index.lock is left behind and the next git command chokes on it. This is the same problem that 98801da fixed by no longer killing git processes; the ConPTY support added in 0.63 reintroduced it through the close event. Disable the automatic refresh for pty-rendered commands. They can afford it: the refresh only persists refreshed stat information, and lazygit's foreground git status refreshes -- which never run in a pty and are never killed -- already write that back on every user action and on terminal focus-in. The cost is that while the on-disk stat cache is stale, an external differ is invoked even for files whose stat information changed but whose content didn't, showing them as empty diffs; this heals with the next foreground refresh, which also re-renders the view. Unix keeps the refresh: a stopped pty child gets SIGTERM there, and git's signal handlers remove its lock files, so the lock window is harmless. The rawGit renderer keeps it too: its tasks don't run in a pty and are never killed on Windows -- they either run to completion or die on a broken pipe mid-output, before the refresh begins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ty on Windows Pty commands are not always direct git invocations: user-configured commands like branchLogCmd can be arbitrary command lines, e.g. wrapping git in 'sh -c' to pipe the log through sed. Injecting git flags after argv[0] of such a command corrupts it (the shell tries to run the config value as its -c script). Only inject into direct git invocations; the wrapped commands are log commands, which never take the index lock, so the protection isn't needed there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ty on Windows Re-wrap the doc comment; the previous fixup left ragged line breaks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
At the end of a diff against the worktree, git re-reads and refreshes the index and writes it back if it found stale stat information (diff.autoRefreshIndex, on by default). It holds index.lock for the whole refresh; GIT_OPTIONAL_LOCKS does not cover this lock, and the window scales with the size of the repository (~150ms for a 6k-file repository with a warm stat cache).
On Windows, a pty task that is stopped because the user moved on terminates its git process at an arbitrary point: tearing down the pseudoconsole delivers CTRL_CLOSE_EVENT, which git leaves to the default handler, which simply calls ExitProcess. If that lands inside the refresh, a stale index.lock is left behind and the next git command chokes on it. This is the same problem that 98801da fixed by no longer killing git processes; the ConPTY support added in 0.63 reintroduced it through the close event.
Disable the automatic refresh for pty-rendered commands. They can afford it: the refresh only persists refreshed stat information, and lazygit's foreground git status refreshes -- which never run in a pty and are never killed -- already write that back on every user action and on terminal focus-in. The cost is that while the on-disk stat cache is stale, an external differ is invoked even for files whose stat information changed but whose content didn't, showing them as empty diffs; this heals with the next foreground refresh, which also re-renders the view.
Unix keeps the refresh: a stopped pty child gets SIGTERM there, and git's signal handlers remove its lock files, so the lock window is harmless. The rawGit renderer keeps it too: its tasks don't run in a pty and are never killed on Windows -- they either run to completion or die on a broken pipe mid-output, before the refresh begins.