registry: "send on closed channel" panic when a model generation times out
- Dominant language
- Go
- Stars
- 223
- Forks
- 225
- Avg merge
- 7d 1h
- Merged PRs (30d)
- 1
Description
`InvokeGenerationFromSheetWithOptions` (`registry/model.go`) has a goroutine-lifecycle race that can panic with `send on closed channel` and crash the whole process when a model's generation exceeds `opts.ModelTimeout`. It is interleaving-dependent, not a crash on every timeout: it fires only when an orphaned worker goroutine reaches its send after the channel has been closed. A run where the worker happens to send before the close just leaks the goroutine instead.
**Mechanism**
For each model an outer goroutine is started and tracked with `wg.Add(1)` (line 899). Inside it, a second inner goroutine (line 917) does the actual generation work (`generator.GetPackage()`, `GenerateComponents()`, the filesystem writes) and sends its result on the unbuffered `spreadsheeetChan` at line 1024.
The outer goroutine then waits on a `select` (line 1038): either the inner goroutine finishes (`<-done`) or the per-model timeout fires (`<-modelCtx.Done()`, line 1055). On timeout the outer goroutine returns and its deferred `wg.Done()` runs, but the inner goroutine is not tracked by `wg` and is not cancelled: neither the work nor the send at 1024 ever checks `modelCtx`. It keeps running.
Once every model has finished or timed out, `wg.Wait()` (line 1065) returns and `close(spreadsheeetChan)` runs (line 1066). An inner goroutine still alive from a timed-out model that later reaches line 1024 sends on the now-closed channel, which panics. A goroutine panic is fatal, so the whole process crashes, and it happens after the run has already logged its completion summary, which makes the crash hard to trace back to its cause.
Even when it does not panic, it is a goroutine and fd leak: the orphaned inner goroutine keeps doing filesystem writes that nothing is tracking.
**Trigger**
Any model whose generation exceeds `ModelTimeout` (default is 5 minutes via `DefaultModelTimeout`). This is realistic under github / artifacthub rate limiting or with large sources, which is the exact situation the timeout was added to bound. `generator.GetPackage()` takes no context and cannot be cancelled, so a slow inner goroutine genuinely outlives the outer one.
**Suggested direction**
The inner goroutine's send has to be safe against both cancellation and the channel already being closed, because the producer can outlive the close (generation is not cancellable). Options: a context-aware send that also cannot panic if the channel is closed, or only closing `spreadsheeetChan` after every producer is accounted for. Happy to open a PR once the preferred approach is clear.
Contributor guide
Research direction
Start in registry/model.go at InvokeGenerationFromSheetWithOptions, especially the inner generation goroutine, its send near line 1024, the timeout select, and channel close near lines 1065-1066. Trace how timed-out workers are accounted for, then reproduce a generation exceeding ModelTimeout; done means no send-on-closed-channel panic and no untracked worker continuing after completion.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100