Skip to content

fix: goroutine leak in UsenetReader.Close() and Go idiom modernization - #429

Merged
javi11 merged 3 commits into
mainfrom
fix/context-propagation-and-error-handling
Mar 21, 2026
Merged

fix: goroutine leak in UsenetReader.Close() and Go idiom modernization#429
javi11 merged 3 commits into
mainfrom
fix/context-propagation-and-error-handling

Conversation

@javi11

@javi11 javi11 commented Mar 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Fix goroutine leak: Add second cond.Broadcast() after clearing rg on the timeout path in UsenetReader.Close(), waking goroutines that entered cond.Wait() after the initial broadcast (confirmed via pprof: 26 stuck downloadManager + 25 stuck close-waiter goroutines)
  • Go idiom modernization (go fix ./...): bytes.SplitSeq, strings.Cut/CutPrefix/CutSuffix, remove unnecessary loop variable captures, simplify min/max expressions

Test plan

  • go build ./... passes
  • go test ./internal/usenet/... passes
  • Verify via pprof that stuck goroutines no longer accumulate after repeated file close timeouts

🤖 Generated with Claude Code

Co-author @firestaerter3

javi11 and others added 3 commits March 20, 2026 21:21
Use the reader's own context (b.ctx) instead of context.Background() in
UsenetReader.Close() so the log call respects graceful shutdown.

Handle previously-ignored LastInsertId() errors in AddToQueue and
AddBatchToQueue across both QueueRepository and Repository, and
deduplicate the rg.Clear() cleanup in Close() that was duplicated
across both select branches.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…de quality

- Use subtle.ConstantTimeCompare for download key authentication to prevent timing attacks
- Add io.LimitReader (50MB) cap on Prowlarr NZB downloads to prevent DoS
- Validate Stremio streamType against whitelist and check strconv.Atoi errors
- Extract duplicate hashAPIKey to shared internal/auth package
- Replace fmt.Printf with structured slog calls in config/manager.go
- Convert ~38 slog calls to context-aware variants across 7 files
- Store context in rclone decrypter struct to replace context.TODO()
- Defer file.Close() in pathutil to prevent leak on Write panic

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add second cond.Broadcast() after clearing rg on the timeout path in
UsenetReader.Close() to wake goroutines that entered cond.Wait() after
the initial broadcast, preventing goroutine accumulation.

Also apply go fix modernizations: bytes.SplitSeq, strings.Cut/CutPrefix/
CutSuffix, remove unnecessary loop variable captures, and simplify
min/max expressions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@javi11 javi11 changed the title fix: propagate context and handle LastInsertId errors fix: goroutine leak in UsenetReader.Close() and Go idiom modernization Mar 21, 2026
@javi11
javi11 merged commit 7f5cfc8 into main Mar 21, 2026
2 checks passed
@javi11
javi11 deleted the fix/context-propagation-and-error-handling branch March 21, 2026 18:46
yoshitaka420 pushed a commit to yoshitaka420/altmount that referenced this pull request Jun 1, 2026
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