Skip to content

registry: "send on closed channel" panic when a model generation times out #1102

Description

@Atishyy27

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions