Skip to content

Bound git stdout/stderr capture to prevent GVFS.Mount OOM#2048

Merged
tyrielv merged 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/bound-git-output-buffer
Jul 9, 2026
Merged

Bound git stdout/stderr capture to prevent GVFS.Mount OOM#2048
tyrielv merged 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/bound-git-output-buffer

Conversation

@tyrielv

@tyrielv tyrielv commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

GVFS.Mount could crash with System.OutOfMemoryException. GitProcess.InvokeGitImpl buffered all of a git process's stdout and stderr into unbounded StringBuilders via the DataReceived handlers. A corrupt/truncated packfile in the shared object cache makes git multi-pack-index stream a large volume of could not load pack errors to stderr, growing the buffer until the mount OOM-crashes.

This is the first of a series that splits a larger fix into independently-reviewable, lower-risk pieces:

  • This PR — bound the captured output (the crash-stopper).
  • Follow-up — convert the two -z commands below to stream their output (removes the truncation exposure entirely).
  • Separate PR — packfile-corruption auto-recovery.

Change

Bound each captured stream with its own cap:

  • stderr: ~20 MB (10M chars). stderr is diagnostic, so truncating it is safe — this is the cap that actually stops the corrupt-packfile crash.
  • stdout: ~256 MB (128M chars) when the caller buffers the whole result. Sized to hold the machine-readable (-z) output of the largest real repositories — on the order of 2.8M status/diff records on the biggest os.2020 branches — with headroom, while staying far below the .NET max array/string size so the buffer itself cannot OOM. Line-streamed commands are unaffected.

Truncation is surfaced on GitProcess.Result (OutputTruncated, ErrorsTruncated) so callers can react:

  • stdout is correctness-critical for two commands, which now fail safe on truncation + emit telemetry rather than act on a partial result:
    • AddStagedFilesToModifiedPaths (diff --cached --name-status -z) — refuses to update ModifiedPaths from a partial staged-file list (which would leave staged files with skip-worktree set and stale placeholders).
    • SparseVerb git status check (status --porcelain -z) — aborts sparse rather than proceed over uncommitted changes it failed to see.
  • stderr truncation is expected and non-fatal; GitMaintenanceStep.RunGitCommand logs it for diagnostics but does not fail the command for it.

Tests

GitProcessTests: BoundedGitOutputBuffer keeps short output, truncates a flood without unbounded growth (with a marker + Truncated flag), keeps the partial line that crosses the cap, and does not trip exactly at the cap; Result truncation flags default to false and round-trip through the constructor. Full unit suite: 882 passed, 0 failed (11 pre-existing native-hook skips).

Note on follow-up

The two -z callers keep their buffered form here; the fail-safe valves are the interim guard. The follow-up PR converts them to NUL-delimited streaming (no buffering, no truncation possible) and removes the valves. A functional test for the large-unstage path is tracked for that PR.

GVFS.Mount could crash with OutOfMemoryException. GitProcess.InvokeGitImpl
buffered all of a git process's stdout and stderr into unbounded StringBuilders
via the DataReceived handlers. A corrupt or truncated packfile in the shared
object cache makes 'git multi-pack-index' stream a large volume of "could not
load pack" errors to stderr, growing the buffer until the mount OOM-crashes.

Bound each captured stream with its own cap:

  - stderr: ~20 MB (10M chars). stderr is diagnostic, so truncating it is safe;
    this is the cap that stops the corrupt-packfile crash.
  - stdout: ~256 MB (128M chars) when the caller buffers the whole result.
    Sized to hold the machine-readable (-z) output of the largest real
    repositories - on the order of 2.8M status/diff records on the biggest
    os.2020 branches - with headroom, while staying far below the .NET maximum
    array/string size so the buffer itself cannot OOM. Commands that stream
    their output line-by-line are unaffected.

Truncation is surfaced on GitProcess.Result via OutputTruncated and
ErrorsTruncated so callers can react:

  - stdout carries correctness-critical data for two commands, which now fail
    safe on truncation rather than act on a partial result:
      - AddStagedFilesToModifiedPaths (git diff --cached --name-status -z):
        refuses to update ModifiedPaths from a partial staged-file list, which
        would otherwise leave staged files with skip-worktree set and stale
        placeholders.
      - SparseVerb's git status check (status --porcelain -z): aborts sparse
        rather than risk proceeding over uncommitted changes it failed to see.
    Both emit telemetry on truncation.
  - stderr truncation is expected and non-fatal; GitMaintenanceStep.RunGitCommand
    logs it for diagnostics but does not fail the command for it.

This is the first of a series: a follow-up converts those two -z commands to
stream their output (removing the truncation exposure entirely), and packfile
corruption auto-recovery is a separate change.

Assisted-by: Claude Opus 4.8
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
@tyrielv tyrielv merged commit 6d92f79 into microsoft:master Jul 9, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants