This is an automated email from the ASF dual-hosted git repository. sjanc pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mynewt-newtmgr.git
commit e0779ce4e09149408ff9d2a1b66dcd8af08f1531 Author: Szymon Janc <[email protected]> AuthorDate: Thu Oct 22 00:35:17 2026 +0200 nmxact/nmxutil: Fix ErrFunnel deadlock when timer already fired When a more severe error was inserted, Insert() stopped the accumulation timer and, if Stop() reported that the timer had already fired, tried to drain errTimer.C. The timer is created with time.AfterFunc(), whose C is nil, so the receive blocked forever while holding the funnel lock. The pending timerExp() callback then blocked on the same lock, so neither the inserting goroutine nor any waiter ever made progress. Only re-arm the timer if it was stopped before firing; otherwise the already running timerExp() picks up the new error once Insert() releases the lock. Assisted-by: Claude:claude-opus-5-5 --- nmxact/nmxutil/err_funnel.go | 11 +++++++---- nmxact/nmxutil/sync_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/nmxact/nmxutil/err_funnel.go b/nmxact/nmxutil/err_funnel.go index baba843..cc6dbd7 100644 --- a/nmxact/nmxutil/err_funnel.go +++ b/nmxact/nmxutil/err_funnel.go @@ -55,11 +55,14 @@ func (f *ErrFunnel) Insert(err error) { }) } else { if f.LessCb(f.curErr, err) { - if !f.errTimer.Stop() { - <-f.errTimer.C - } f.curErr = err - f.errTimer.Reset(f.AccumDelay) + + // The timer was created by time.AfterFunc(), so it has no + // channel to drain. If it already fired, timerExp() is + // waiting for the lock and will report the new error. + if f.errTimer.Stop() { + f.errTimer.Reset(f.AccumDelay) + } } } } diff --git a/nmxact/nmxutil/sync_test.go b/nmxact/nmxutil/sync_test.go index 0245229..aee7e51 100644 --- a/nmxact/nmxutil/sync_test.go +++ b/nmxact/nmxutil/sync_test.go @@ -455,3 +455,38 @@ func TestErrFunnelNilInsert(t *testing.T) { }() f.Insert(nil) } + +// A more severe error inserted just as the accumulation timer fires must +// not deadlock Insert (time.AfterFunc timers have no channel to drain) +// and must still be the error reported. +func TestErrFunnelInsertWhileTimerFires(t *testing.T) { + f := &ErrFunnel{ + AccumDelay: time.Millisecond, + } + f.LessCb = func(a error, b error) bool { + // Called with the funnel locked: give the timer time to fire so + // that Stop() fails. If it does not fire in time the test still + // passes, it just does not exercise the race. + time.Sleep(50 * time.Millisecond) + return sevLess(a, b) + } + + w := f.Wait() + f.Insert(sevError(1)) + + done := make(chan struct{}) + go func() { + f.Insert(sevError(2)) + close(done) + }() + + select { + case <-done: + case <-time.After(syncTestTimeout): + t.Fatalf("Insert deadlocked") + } + + if err := recvFunnel(t, w); err != sevError(2) { + t.Errorf("got %v, want severity 2", err) + } +}
