yonlu/yellowjacket
1
1
Fork 0
Code Issues 53 Pull Requests Actions Packages Projects Releases 13 Wiki Activity

slskd: a failed or stalled transfer is never retried elsewhere, never cancelled, and can hold the Soulseek slot for 6h #263

New Issue
Closed
opened 2026-09-26 20:03:50 +00:00 by yonlu · 2 comments
yonlu commented 2026-09-26 20:03:50 +00:00
Owner
Copy Link
Copy Source

A failed or stalled Soulseek transfer fails the whole download, can hold the only slskd slot for six hours, and is never cancelled in slskd. Found by reading backend/download/ (no live slskd run).

Defects

  1. No fallback to the next candidate. Manager.grab → transfer → failItem → failDownload (manager.go). On Soulseek the usual failure is one peer offline or refusing, and a popular album has many other peers offering the same folder. The ranked list is already held in m.results, but nothing walks it.
  2. No stall detection. slskd.awaitTransfers exits only when every wanted file is terminal or the context ends. slskd's concurrency is 1 per provider and grabTimeout is 6h, so one peer holding us in Queued, Remotely blocks every Soulseek download for up to six hours.
  3. Cancel / timeout does not cancel the transfer in slskd. On ctx.Done() the adapter returns, and slskd keeps downloading (or stays queued with the peer), leaving orphan files in its downloads directory.
  4. A file missing from the transfer list is waited on forever. A file rejected at enqueue never appears in GET /transfers/downloads/{user}, so done+failed == len(wanted) never holds. Separately, a terminal record left over from an earlier attempt at the same user/file matches by filename and counts immediately.

Direction

  • awaitTransfers gives up on a transfer that makes no progress within a bounded window (still queued remotely, or no bytes moved), and treats a wanted file absent from the list after a grace period as failed.
  • On any non-success exit, the adapter cancels its own outstanding transfers in slskd (DELETE /api/v0/transfers/downloads/{user}/{id}), keyed on the transfer ids it owns rather than on filename.
  • Manager.grab falls through to the next-ranked candidate from a different peer when a transfer fails. The number of fallbacks is bounded, and an unattended auto-pick only falls through to a candidate that would itself have passed AutoPickVeto, so the gates are not weakened by retrying.

Out of scope, noted from the same audit

These are left for separate issues:

  • per-peer rather than per-provider concurrency;
  • multi-disc folder grouping (CD1/CD2 split into two candidates, which also collide in collect);
  • slskdMinFiles = 2 dropping single-track results;
  • titleFit/completeness counting files rather than matched tracks;
  • a query ladder and slskd search options;
  • using the per-file length that is already received for duration matching.
A failed or stalled Soulseek transfer fails the whole download, can hold the only slskd slot for six hours, and is never cancelled in slskd. Found by reading `backend/download/` (no live slskd run). ## Defects 1. **No fallback to the next candidate.** `Manager.grab` → `transfer` → `failItem` → `failDownload` (`manager.go`). On Soulseek the usual failure is one peer offline or refusing, and a popular album has many other peers offering the same folder. The ranked list is already held in `m.results`, but nothing walks it. 2. **No stall detection.** `slskd.awaitTransfers` exits only when every wanted file is terminal or the context ends. slskd's concurrency is 1 per provider and `grabTimeout` is 6h, so one peer holding us in `Queued, Remotely` blocks **every** Soulseek download for up to six hours. 3. **Cancel / timeout does not cancel the transfer in slskd.** On `ctx.Done()` the adapter returns, and slskd keeps downloading (or stays queued with the peer), leaving orphan files in its downloads directory. 4. **A file missing from the transfer list is waited on forever.** A file rejected at enqueue never appears in `GET /transfers/downloads/{user}`, so `done+failed == len(wanted)` never holds. Separately, a terminal record left over from an earlier attempt at the same user/file matches by filename and counts immediately. ## Direction - `awaitTransfers` gives up on a transfer that makes no progress within a bounded window (still queued remotely, or no bytes moved), and treats a wanted file absent from the list after a grace period as failed. - On any non-success exit, the adapter cancels its own outstanding transfers in slskd (`DELETE /api/v0/transfers/downloads/{user}/{id}`), keyed on the transfer ids it owns rather than on filename. - `Manager.grab` falls through to the next-ranked candidate from a **different** peer when a transfer fails. The number of fallbacks is bounded, and an unattended auto-pick only falls through to a candidate that would itself have passed `AutoPickVeto`, so the gates are not weakened by retrying. ## Out of scope, noted from the same audit These are left for separate issues: - per-peer rather than per-provider concurrency; - multi-disc folder grouping (`CD1`/`CD2` split into two candidates, which also collide in `collect`); - `slskdMinFiles = 2` dropping single-track results; - `titleFit`/completeness counting files rather than matched tracks; - a query ladder and slskd search options; - using the per-file `length` that is already received for duration matching.
yonlu added the
Priority
High
2
Platform/DesktopArea/DownloadsKind/Bug labels 2026-09-26 20:03:50 +00:00
yonlu self-assigned this 2026-09-26 20:03:57 +00:00
yonlu added the
Status
In Progress
 label 2026-09-26 20:03:57 +00:00
yonlu commented 2026-09-26 20:03:58 +00:00
Author
Owner
Copy Link
Copy Source

Taking this in a worktree on fix/263-slskd-transfer-lifecycle. Approach: the adapter records the transfer ids it enqueued, detects a stall (still queued with no progress within a window, or a wanted file absent from the list after a grace period), and cancels its own transfers in slskd on every non-success exit. Manager.grab then falls through to the next-ranked candidate from a different peer — bounded, and for unattended picks only to candidates that clear the same veto.

Taking this in a worktree on `fix/263-slskd-transfer-lifecycle`. Approach: the adapter records the transfer ids it enqueued, detects a stall (still queued with no progress within a window, or a wanted file absent from the list after a grace period), and cancels its own transfers in slskd on every non-success exit. `Manager.grab` then falls through to the next-ranked candidate from a different peer — bounded, and for unattended picks only to candidates that clear the same veto.
yonlu commented 2026-09-26 20:12:19 +00:00
Author
Owner
Copy Link
Copy Source

Two commits on fix/263-slskd-transfer-lifecycle (not pushed yet):

  • fc012122: the slskd adapter now gives up after 10 minutes with no bytes moving, counts a file slskd never lists as failed after a 30s grace, ignores terminal records that existed before the enqueue, and cancels and removes its live transfers on every non-success exit. Usernames are now path-escaped.
  • 0a33b9d6: Manager.grab falls through to the next auto-acceptable candidate, up to 3 attempts. On Soulseek a failed peer rules out every folder it offered. A hand-picked candidate does not fall back.

One scope addition, found while doing this: Start and Attempt grabbed ranked[0], while AutoPickVeto had judged the first candidate inside the guardrails. So when the overall best candidate was over MaxSizeMB, auto-pick downloaded it anyway. autoPick now returns the candidate the veto actually judged, and this is covered by TestManagerAutoPickTakesTheBestEligibleCandidate.

Verification: go test -race -count=3 ./backend/download/ passes, and golangci-lint reports 0 issues under the default and dev tags. There has been no run against a live slskd.

Two commits on `fix/263-slskd-transfer-lifecycle` (not pushed yet): - `fc012122`: the slskd adapter now gives up after 10 minutes with no bytes moving, counts a file slskd never lists as failed after a 30s grace, ignores terminal records that existed before the enqueue, and cancels and removes its live transfers on every non-success exit. Usernames are now path-escaped. - `0a33b9d6`: `Manager.grab` falls through to the next auto-acceptable candidate, up to 3 attempts. On Soulseek a failed peer rules out every folder it offered. A hand-picked candidate does not fall back. **One scope addition, found while doing this:** `Start` and `Attempt` grabbed `ranked[0]`, while `AutoPickVeto` had judged the first candidate *inside* the guardrails. So when the overall best candidate was over `MaxSizeMB`, auto-pick downloaded it anyway. `autoPick` now returns the candidate the veto actually judged, and this is covered by `TestManagerAutoPickTakesTheBestEligibleCandidate`. Verification: `go test -race -count=3 ./backend/download/` passes, and golangci-lint reports 0 issues under the default and `dev` tags. There has been no run against a live slskd.
yonlu closed this issue 2026-09-27 01:45:21 +00:00
gitea-actions bot removed the
Status
In Progress
 label 2026-09-27 01:46:30 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Area/Design
Area/Downloads
Area/Explore
Area/Library-UI
Area/Metadata
Area/Packaging
Area/Player
Area/Queue
Area/Settings
Area/Shell-Nav
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
 Platform/Android
Platform/Desktop
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Medium
3
The priority is medium
Priority
Low
4
The priority is low
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
In Progress
Somebody is actively working on this right now
No labels Area/Downloads Kind/Bug Platform/Desktop
Priority
High
2
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
logan yonlu
No Assignees yonlu
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: yonlu/yellowjacket#263