Skip to content

fix(git): kill upload-pack when the client disconnects - #47

Open
Temikus wants to merge 2 commits into
rolandjitsu:mainfrom
Temikus:fix/kill-upload-pack-on-disconnect
Open

Temikus wants to merge 2 commits into
rolandjitsu:mainfrom
Temikus:fix/kill-upload-pack-on-disconnect

Conversation

@Temikus

@Temikus Temikus commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

  • fix
  • feat
  • refactor
  • perf
  • docs
  • test
  • build / ci
  • chore

Summary
Point 3 follow-up from #45. When a client hangs up mid-serve, upload-pack and its pack-objects child kept running. pack-objects writes nothing while it counts and compresses, so it never gets SIGPIPE and can pack for minutes with nobody reading.

upload-pack now runs in its own process group. TimedReader signals a oneshot at EOF; if the stream is dropped before that, the reaper SIGKILLs the group before reaping, so the group ID cannot be recycled. The reaper is spawned before the request write, so a request dropped mid-write is cleaned up too.

rustix (unix only) is a new direct dependency for a safe killpg. It was already in the lockfile via tempfile.

Tests

  • Added / updated tests (unit, plus integration where it fits)

The e2e test uses a fake git_binary whose upload-pack starts a silent grandchild, then drops the TCP connection and checks the grandchild dies. It fails on main and passes here.

Checklist

  • CI is green locally: cargo fmt --all --check, cargo clippy --all-targets --all-features --locked -- -D warnings, cargo test --all-features
  • Commits follow Conventional Commits; AI-assisted commits carry an Assisted-by: trailer (see CONTRIBUTING.md / AGENTS.md)
  • Preserves the read-only, pull-only invariant (no push or proactive replication to upstream)
  • Docs / README updated if behavior or flags changed

Breaking change?
No.

@codecov

codecov Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.94737% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.93%. Comparing base (66a0417) to head (facb844).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/git.rs 98.94% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #47      +/-   ##
==========================================
+ Coverage   96.80%   96.93%   +0.12%     
==========================================
  Files           6        6              
  Lines        2065     2151      +86     
==========================================
+ Hits         1999     2085      +86     
  Misses         66       66              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pack-objects writes nothing while counting, so it never got SIGPIPE and
kept packing for minutes after the client left.

Assisted-by: Claude:claude-opus-5-5
Exiting without reading raced the query write into EPIPE, so the
cat-file failure branch was hit only some of the time.

Assisted-by: Claude:claude-opus-5-5
@Temikus
Temikus force-pushed the fix/kill-upload-pack-on-disconnect branch from 1d924df to facb844 Compare October 13, 2026 01:11
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.

1 participant