Horde Storage GC Sweeper Can Deadlock Permanently

We ran into a problem where Horde stopped cleaning up build artifacts on our storage backend (a filesystem/NAS backend reached over SMB), and disk usage kept climbing. Investigating, we confirmed artifact retention (keepDays/keepCount) was working correctly at the metadata/API level — expired artifacts disappeared from Horde’s API as expected. The physical blob files on disk, however, were never actually being deleted.

The cause is a bug in the Storage:GC ticker (StorageService.cs), the background job responsible for physically deleting blobs once nothing references them anymore. It’s registered as a single, sequential ticker that only schedules its next run after the previous one fully returns — there is no per-tick timeout or watchdog anywhere in that loop.

Tracing the call chain: TickGcAsync → TickGcForNamespaceAsync → AsyncPipeline.WaitForCompletionAsync(), which does Task.WhenAll over a pool of parallel reachability-check workers and only returns once all of them finish. Each worker eventually calls namespaceInfo.Store.DeleteAsync(…). For a filesystem-backed namespace this resolves to FileObjectStore.DeleteAsync, which turns out to be a fully synchronous method under the hood (File.Delete under a lock, plus a directory-cleanup walk) that accepts a CancellationToken but never actually checks it anywhere in the chain.

If the SMB session to the NAS stalls even briefly — a known failure mode for File.Delete/Directory.Delete against a degraded UNC path — that synchronous call blocks its thread indefinitely, and cannot be interrupted by cancelling a token, because nothing downstream observes one. Since Task.WhenAll won’t complete until every worker returns, one single stuck delete permanently wedges the entire ticker — not just for the affected namespace, but for every namespace on the server, since they all share the same ticker. No exception is ever thrown, so nothing gets logged, and there’s no automatic recovery path. In our case the sweeper had silently been dead for 12+ days across all namespaces before we noticed rising disk usage and started digging. Restarting the Horde server cleared it instantly, which confirms the diagnosis but is obviously not a real fix — the exact same thing can recur any time the NAS has a transient hiccup, with no warning before disk fills up again.

We think the right scope for a fix is the specific call site, not the shared FileObjectStore/underlying storage code (which is used well beyond just Horde’s GC path). A CancellationToken-based timeout won’t work here, since nothing downstream ever reads it — the fix has to race the delete call with a separate timer instead, and on timeout just leave that blob queued for the next sweep rather than blocking forever.

We’d appreciate knowing whether this is a known issue on your side, whether there’s a preferred way of handling it, or whether our proposed fix below looks reasonable before we commit to implementing it.

Proposed fix

// StorageService.cs, CheckReachabilityAsync, around the existing Store.DeleteAsync call
if (storageConfig.EnableGc)
{
    Task deleteTask = namespaceInfo.Store.DeleteAsync(objectKey, cancellationToken);
    Task winner = await Task.WhenAny(deleteTask, Task.Delay(TimeSpan.FromSeconds(30), cancellationToken));
    if (winner != deleteTask)
    {
        _logger.LogWarning(
            "Timed out deleting {NamespaceId} blob {BlobId} from storage (possible stalled I/O); leaving queued for retry next sweep",
            namespaceInfo.Id, blobInfoId);
        return; // don't mark as deleted; blob stays in the check queue and will be retried
    }


    try
    {
        await deleteTask; // guaranteed complete now; surface any real exception
    }
    // existing missing-file catch block stays as-is
}

[Attachment Removed]

Hello! Thank you for reporting this and proposing a fix. I’ve not been able to find any matching tickets on our end for this, so it seems like we don’t have it tracked yet.

Generally, the approach makes sense, but I can see two issues. Firstly, I think the current implementation would still block on the namespaceInfo.Store.DeleteAsync() call.

If instead you wrapped the call with Task.Run(), the Task.WhenAny() call can then be reached.

Task deleteTask = Task.Run(() => namespaceInfo.Store.DeleteAsync(objectKey, CancellationToken.None), CancellationToken.None);
Task winner = await Task.WhenAny(deleteTask, Task.Delay(TimeSpan.FromSeconds(30), cancellationToken));

Secondly, I’m not sure how this would work in practice, with delete threads leaking in the background and becoming abandoned tasks.

We’d probably need to look at how we could cancel these.

Let me know what you think and if you’ve been able to test/observe this in action yet.

For the report, would you like me to submit this bug to the team, or would you like to open a PR so that we can properly attribute you with the fix?

[Attachment Removed]

Agreed, with the open questions and the likelihood that the surface area of this fix will expand, that sounds like the best plan.

I’ll raise this with the Horde engineering team as a bug and let you know if there are any immediate updates.

[Attachment Removed]

Thanks for the catch on Task.Run — you’re right, without it the blocking call happens before Task.WhenAny is even reached, so the current version wouldn’t actually fix the deadlock. Good call.

On the leaked/abandoned tasks — that’s a fair concern and we don’t have a solid answer for it yet. The two things that come to mind on our side:

  1. Bound how many of these timed-out deletes can be in flight at once (e.g. a semaphore), so a run of stalls can’t accumulate unboundedly — once the limit is hit, GC just stops attempting further deletes until some of the outstanding ones resolve.
  2. Push the fix down a level: shorten the SMB/network timeout on the client side so a stalled
  3. File.Delete
  4. against a degraded UNC path fails fast with an exception instead of hanging indefinitely. That would shrink the leaked-task risk on its own, though it’s more of an infra change than a code fix.

We haven’t tested/observed either of these in practice yet, so if you have a preferred approach (or have run into this class of problem before), we’d rather go with that.

Given there’s still an open question here, we think it makes more sense to file this as a bug report for now rather than push a PR for a fix we’re not fully confident in — happy to follow up with a PR once we land on something solid.

[Attachment Removed]