A staging flush that fails (e.g. disk full) turns an upload upstream accepted into a 500, and leaves the staging file uncollectable until restart

[matt-edmondson]

What's wrong

ObjectStore.PublishAsync calls StagingHandle.CloseAsync outside any try/catch (GitLfsCache/Storage/ObjectStore.cs:128). CloseAsync flushes and disposes the staging stream (GitLfsCache/Storage/StagingHandle.cs:58-66).

FileStream buffers writes smaller than its 4 KiB buffer, so on a full disk the IOException can surface only at the final FlushAsync and not on any write. HashingStream.Faulted only tracks failed writes, so the existing faulted-handle branch (added for #45/#59) never sees this case. When the flush throws, two things go wrong:

1. The request fails after upstream accepted it. In ObjectRouteHandler.UploadAsync (Endpoints/ObjectRouteHandler.cs:250-263), PublishAsync runs after upstream answered 2xx but before UpstreamRelay.CopyResponseAsync. The exception escapes, so the client gets a 500 for a push upstream accepted. The download path throws after the body has already streamed. Both break the design's rule that a store write failure degrades to plain pass-through (gitlfscache-design spec, Failure handling).

2. The staging file leaks until restart. The await using then calls StagingHandle.DisposeAsync, which:

   • sets _disposed = true (line 77), then
   • calls _stream.DisposeAsync() (line 78). FileStream.DisposeAsync retries the failed flush and throws again.

   That second throw skips both _onClosed (line 82, which removes the path from ObjectStore._openStaging) and the file delete. Since _disposed is already true, nothing retries. ObjectStore.TryDeleteStaging (ObjectStore.cs:303-309) refuses any path still in _openStaging, so the maintenance sweep skips that file forever. Every flush failure on an already-full disk adds another file the sweep can't delete, which makes the full disk worse.

Evidence

• A real FileStream on /dev/full: a 100-byte WriteAsync succeeds (it is buffered), FlushAsync throws IOException: No space left on device, the first DisposeAsync throws again, and a second DisposeAsync succeeds.
• A scratch test drove ObjectStore.PublishAsync with a staging sink that behaves the same way (buffered, throws on flush and on the first dispose). Result: publish threw: IOException; dispose threw: IOException; onClosed called: False; staging exists: True.

Suggested fix

• In StagingHandle.CloseAsync, catch IOException and UnauthorizedAccessException from the flush and dispose, and mark the handle faulted. PublishAsync then discards it through the existing Faulted branch and returns false.
• In StagingHandle.DisposeAsync, run _onClosed and the file delete in a finally (or catch the stream-dispose exception), so the guard always lifts and cleanup always runs.

Acceptance criteria

With a staging sink that throws on flush and on the first dispose:

• PublishAsync returns false and does not throw.
• The staging path leaves the open-staging set, and the file is deleted (or collected by the next sweep).
• An upload through the proxy still returns upstream's status to the client.

Related but distinct: #57 / PR #68 cover OpenStaging itself throwing.

[matt-edmondson]

Triage

• Category: Bug
• Priority: High. When a disk fills up, a push upstream already accepted comes back to the client as a 500. This breaks the design rule that store failures fall back to pass-through. Each failure also leaks a staging file the sweep can never collect, so the full disk gets worse.
• Area / suggested assignee: Storage layer (Storage/StagingHandle.cs, Storage/ObjectStore.cs), @matt-edmondson.
• Duplicates: None. Related but distinct: When the staging file can't be created, pulls and pushes fail with 500 instead of bypassing the cache #57 / PR Relay transfers uncached when no staging file can be opened #68 (OpenStaging itself throwing) and A disk-full error on the final chunk of a download publishes a truncated object that passes hash verification #45/Refuse to publish a staging file whose write failed #59 (faulted writes).
• In progress: No open PR covers it.
• Notes: The reproduction on /dev/full is concrete. The fix is local: catch in CloseAsync and mark the handle faulted, and run _onClosed and the delete in a finally in DisposeAsync.

---

Generated by Claude Code