Skip to content

start_download failure permanently leaks a concurrency slot, active entry, and DB row #393

Description

@forkwright

Finding

In the spawned task inside try_dispatch_next, a failed engine.start_download is only logged. The active map entry, the slot held by the prior allocator.acquire (and its active_total increment), and the DB row set to downloading are all left in their mutated state with no cleanup path.

Evidence

crates/syntaxis/src/lib.rs:361:

if let Err(e) = engine.start_download(request).await {
    error!(error = %e, "failed to dispatch download to engine");
}

The spawned task returns after logging. The active entry inserted at line 329, the slot acquired via allocator.acquire, and the DB row set to downloading at line 352 are never reverted — active.remove, allocator.release, and repo::mark_failed are never called.

Why this matters

Every start_download failure permanently consumes one slot and one active entry. With max_concurrent = N, N consecutive dispatch failures permanently halt all further downloads — a self-inflicted denial of service with no operator-visible recovery. The DB row stays downloading forever, so restart recovery reloads it and re-fails identically, perpetuating the wedge. A capable adversary who can induce engine-dispatch failures (e.g. by degrading the engine endpoint) can therefore lock the acquisition pipeline indefinitely with a bounded number of failures.

Desired correction

On start_download error, within the spawned task call allocator.release, active.remove, and repo::mark_failed (or re-queue for retry), then call try_dispatch_next to unblock the queue. Done when: a test injecting a start_download error verifies the active slot count is unchanged from baseline and the DB row is marked failed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions