Stream large -z git output instead of buffering it against the cap - #2045
Draft
tyrielv wants to merge 1 commit into
Draft
Stream large -z git output instead of buffering it against the cap#2045tyrielv wants to merge 1 commit into
tyrielv wants to merge 1 commit into
Conversation
tyrielv
force-pushed
the
tyrielv/fix-invokegit-stderr-oom
branch
2 times, most recently
from
July 9, 2026 21:37
c0cca88 to
e4198f0
Compare
tyrielv
changed the title
tyrielv
force-pushed
the
tyrielv/fix-invokegit-stderr-oom
branch
2 times, most recently
from
July 10, 2026 17:32
4b8cc4a to
1a2dc13
Compare
Building on the git-output bounding change, this removes the truncation exposure
for the two commands whose full result is correctness-critical by streaming
their output instead of buffering it, while keeping a runtime-selectable safe
fallback for v2.0 stabilization.
DiffCachedNameStatus (diff --cached --name-status -z) feeds every staged path
into ModifiedPaths; StatusPorcelain (status --porcelain -z) drives the sparse
dirty check. Both use -z (NUL-delimited), so line-based streaming cannot chunk
them - git delivers the whole blob as a single line.
Add a NUL-delimited streaming mode to InvokeGitImpl: a new parseStdOutToken
callback reads stdout synchronously and splits on NUL, invoking the callback
once per record as it arrives. stderr stays async (BeginErrorReadLine), so the
synchronous stdout read cannot deadlock. Only one record is held in memory, so
an arbitrarily large result streams without buffering, truncation, or OOM.
Both commands expose a streaming overload and a buffered overload. The callers
choose at runtime from the gvfs.stream-git-status-output kill switch (default
true): when disabled they use the bounded-buffer path and its OutputTruncated
fail-safes, so streaming can be turned off in the field without a redeploy.
Add an optional streaming watchdog gated by gvfs.git-status-stream-timeout-seconds
(default -1 = infinite/disabled): when set, a timer kills the git process tree if
the synchronous read does not finish in time and the result reports a timeout.
The default is infinite so a legitimately long status on a very large working
tree is never killed. The watchdog disarms under processLock once the read
completes, so a late callback cannot report a false timeout or kill a reused
process.
Hardening from self-review: the streaming read kills the git child if a callback
throws (no orphaned process); AddStagedFilesToModifiedPaths fails on an unpaired
trailing status token rather than acting on an incomplete list; MockGitProcess
feeds output through the production tokenizer so the test double cannot drift.
Tests:
- ReadStdOutTokens: NUL splitting, empty input, embedded empty records, a lone
NUL, a trailing record without a NUL, and a record spanning the 8KB buffer.
- Streaming and buffered overloads of DiffCachedNameStatus/StatusPorcelain.
- GetNextGitPath (buffered fallback) parsing; PathCoveredBySparseFolders
unchanged.
Assisted-by: Claude Opus 4.8
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
tyrielv
force-pushed
the
tyrielv/fix-invokegit-stderr-oom
branch
from
August 10, 2026 23:04
1a2dc13 to
9328962
Compare
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.
Summary
Follow-up to #2048. That PR bounded the previously-unbounded git stdout/stderr capture and, for the two commands whose full result is correctness-critical, added a large stdout cap plus fail-safe valves if the cap was ever exceeded. This PR removes that truncation exposure entirely by streaming those commands' output instead of buffering it.
DiffCachedNameStatus(diff --cached --name-status -z) feeds every staged path intoModifiedPaths.StatusPorcelain(status --porcelain -z) drives the sparse dirty check.Both use
-z(NUL-delimited), so line-based streaming can't chunk them — git delivers the whole blob as a single "line."Change
Add a NUL-delimited streaming mode to
InvokeGitImpl: a newparseStdOutTokencallback reads stdout synchronously and splits on\0, invoking the callback once per record as it arrives. stderr stays async (BeginErrorReadLine), so the synchronous stdout read is deadlock-safe ("one sync, one async"). Only one record is held in memory, so an arbitrarily large result streams with no buffering, truncation, or OOM. It keeps git's robust-zformat (no path-unquoting) and is guarded totimeoutMs == -1and mutual exclusion withparseStdOutLine.DiffCachedNameStatusandStatusPorcelainnow stream tokens; their callers drive small status/path state machines. This removes the interim fail-safe valves from #2048 (there's no longer a partial result to guard against) and the dead-zstring parsers (GetPathsNotCoveredBySparseFolders,GetNextGitPath).Result.OutputTruncated/ErrorsTruncatedremain for the still-buffered commands (stderr is still bounded), but the two converted commands can no longer tripOutputTruncated.Tests
ReadStdOutTokens: NUL splitting, empty input, embedded empty records, a trailing record without a NUL, and a record spanning the 8KB read buffer.DiffCachedNameStatus/StatusPorcelainstream records through the mock.GetNextGitPathtest;PathCoveredBySparseFolderstests unchanged.Full unit suite: 889 passed, 0 failed (11 pre-existing native-hook skips).