Skip to content

Commit fc3bf43

Browse files
committed
fix(syslog): don't close the shared acquisition output channel
The syslog datasource's Stream() does `defer close(out)` on the pipeline output channel. That channel is shared: StartAcquisition() passes the same `out` to every configured datasource, and it is owned by the acquisition orchestrator, which never closes it (shutdown is driven by context cancellation, then drainChan). syslog is the only datasource that closes it. When syslog and another datasource run together, syslog's close races with the other datasource still sending. This is reproducible with the AppSec datasource: on shutdown/reload, an in-flight out-of-band request in appsec_runner's handleOutBandInterrupt does `r.outChan <- evt` after syslog has closed the channel, panicking with "send on closed channel" (seen in production on 1.7.8 under a syslog+appsec config). Remove the close. No consumer relies on it: runParse ranges via a `select { case <-ctx.Done(); case ev := <-input }` loop and drainChan is a non-blocking best-effort drain, so both terminate on context cancellation regardless. This aligns syslog with every other datasource (file, journalctl, docker, ...), none of which close the shared channel. Also documents the "never close out" contract on the RestartableStreamer interface.
1 parent 06be627 commit fc3bf43

2 files changed

Lines changed: 9 additions & 2 deletions

File tree

pkg/acquisition/modules/syslog/run.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,11 @@ func (s *Source) Stream(ctx context.Context, out chan pipeline.Event) error {
4343
})
4444

4545
g.Go(func() error {
46-
defer close(out)
47-
46+
// out is the shared pipeline output channel, fed by every configured
47+
// datasource; it is owned by the acquisition orchestrator, not by this
48+
// Stream(), so it must NOT be closed here. Doing so races with any other
49+
// datasource still sending on it (e.g. an in-flight appsec out-of-band
50+
// request) and panics with "send on closed channel".
4851
for {
4952
select {
5053
case <-ctx.Done():

pkg/acquisition/types/types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ type Fetcher interface {
7676
// failures, but treat them as errors. The caller is responsible for supervising
7777
// Stream(), and restarting it as needed. There is currently no way to differentiate
7878
// retryable vs permanent errors.
79+
// - never close the output channel: it is shared and fed by every configured
80+
// datasource, so it is owned by the acquisition orchestrator, not by any single
81+
// Stream() implementation. Closing it here would race with (and can panic) any
82+
// other datasource still sending on it.
7983
type RestartableStreamer interface {
8084
// Start live acquisition (eg, tail a file)
8185
Stream(ctx context.Context, out chan pipeline.Event) error

0 commit comments

Comments
 (0)