From 300aac9d77ff71dc98364d983550424e10d52d0f Mon Sep 17 00:00:00 2001 From: root Date: Thu, 11 Jun 2026 10:07:30 +0000 Subject: [PATCH 1/2] fix(spray): guard emitStats against cancelled context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SprayEngine.execute calls emitStats in a deferred function. When the caller's context times out, the consumer pipeline may already have shut down by the time the defer fires, so the OnStats callback sends to a closed channel and panics. Check ctx.Err() before invoking statsHandler — if the context is already cancelled the callback is a no-op. Co-Authored-By: Claude Opus 4.6 (1M context) --- spray/context_test.go | 52 +++++++++++++++++++++++++++++++++++++++++++ spray/types.go | 5 +++-- 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 spray/context_test.go diff --git a/spray/context_test.go b/spray/context_test.go new file mode 100644 index 0000000..612c018 --- /dev/null +++ b/spray/context_test.go @@ -0,0 +1,52 @@ +package spray + +import ( + "context" + "sync/atomic" + "testing" + + "github.com/chainreactors/sdk/pkg/types" +) + +func TestEmitStats_NilContext(t *testing.T) { + var c *Context + c.emitStats(types.Stats{}) +} + +func TestEmitStats_NilHandler(t *testing.T) { + c := &Context{ctx: context.Background()} + c.emitStats(types.Stats{}) +} + +func TestEmitStats_ContextCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + var called atomic.Bool + c := &Context{ + ctx: ctx, + statsHandler: func(stats types.Stats) { + called.Store(true) + }, + } + + c.emitStats(types.Stats{Engine: "test"}) + if called.Load() { + t.Fatal("statsHandler must not be called when context is cancelled") + } +} + +func TestEmitStats_ContextActive(t *testing.T) { + var called atomic.Bool + c := &Context{ + ctx: context.Background(), + statsHandler: func(stats types.Stats) { + called.Store(true) + }, + } + + c.emitStats(types.Stats{Engine: "test"}) + if !called.Load() { + t.Fatal("statsHandler should be called when context is active") + } +} diff --git a/spray/types.go b/spray/types.go index 66bc5d8..50dc117 100644 --- a/spray/types.go +++ b/spray/types.go @@ -110,9 +110,10 @@ func (c *Context) SetStatsHandler(handler func(types.Stats)) *Context { } func (c *Context) emitStats(stats types.Stats) { - if c != nil && c.statsHandler != nil { - c.statsHandler(stats) + if c == nil || c.statsHandler == nil || c.ctx.Err() != nil { + return } + c.statsHandler(stats) } // ======================================== From 047b26081dea8b3e19eefdc361f36c67b489a70a Mon Sep 17 00:00:00 2001 From: root Date: Thu, 11 Jun 2026 10:21:14 +0000 Subject: [PATCH 2/2] test: add panic-reproducing test for emitStats after context cancel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestEmitStats_PanicAfterContextCancel simulates the exact w3 crash: context cancelled → consumer closes channel → emitStats defer fires → statsHandler sends on closed channel → panic. Without the ctx.Err() guard this test fails (panic is triggered). With the guard it passes (callback is skipped). Co-Authored-By: Claude Opus 4.6 (1M context) --- spray/context_test.go | 58 +++++++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 18 deletions(-) diff --git a/spray/context_test.go b/spray/context_test.go index 612c018..66b2fa9 100644 --- a/spray/context_test.go +++ b/spray/context_test.go @@ -8,34 +8,57 @@ import ( "github.com/chainreactors/sdk/pkg/types" ) -func TestEmitStats_NilContext(t *testing.T) { - var c *Context - c.emitStats(types.Stats{}) -} - -func TestEmitStats_NilHandler(t *testing.T) { - c := &Context{ctx: context.Background()} - c.emitStats(types.Stats{}) -} - -func TestEmitStats_ContextCancelled(t *testing.T) { +// TestEmitStats_PanicAfterContextCancel reproduces the w3 pipeline panic. +// +// Scenario: consumer sets OnStats callback → context times out → consumer +// shuts down (closes its channel) → SDK defer calls emitStats → callback +// sends to closed channel → panic: send on closed channel. +// +// This test MUST panic before the fix and pass after. +func TestEmitStats_PanicAfterContextCancel(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) - cancel() - var called atomic.Bool + // Simulate pipeline's p.events channel + events := make(chan struct{}, 1) + c := &Context{ ctx: ctx, statsHandler: func(stats types.Stats) { - called.Store(true) + events <- struct{}{} // this panics when events is closed }, } - c.emitStats(types.Stats{Engine: "test"}) - if called.Load() { - t.Fatal("statsHandler must not be called when context is cancelled") + // Step 1: consumer abandons — context cancel + close channel + cancel() + close(events) + + // Step 2: SDK defer fires emitStats after consumer is gone + panicked := true + func() { + defer func() { + if r := recover(); r == nil { + panicked = false + } + }() + c.emitStats(types.Stats{Engine: "spray"}) + }() + + if panicked { + t.Fatal("emitStats called statsHandler after context was cancelled, " + + "causing send on closed channel — this is the bug being fixed") } } +func TestEmitStats_NilContext(t *testing.T) { + var c *Context + c.emitStats(types.Stats{}) +} + +func TestEmitStats_NilHandler(t *testing.T) { + c := &Context{ctx: context.Background()} + c.emitStats(types.Stats{}) +} + func TestEmitStats_ContextActive(t *testing.T) { var called atomic.Bool c := &Context{ @@ -44,7 +67,6 @@ func TestEmitStats_ContextActive(t *testing.T) { called.Store(true) }, } - c.emitStats(types.Stats{Engine: "test"}) if !called.Load() { t.Fatal("statsHandler should be called when context is active")