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 23f9ab3762c1f7472faa2edba1b4c14988670ab8 Author: Szymon Janc <[email protected]> AuthorDate: Thu Oct 22 00:24:00 2026 +0200 nmxact/nmp: Fix race between listener timeout and Close The AfterTimeout callback sent on tmoChan without any synchronization with Close. If the timer fired while the listener was being closed (e.g. a response and the timeout arriving at about the same time), the callback could send on the just-closed channel, crashing newtmgr with "panic: send on closed channel" from nmp.(*Listener).AfterTimeout. The race detector also reports the unsynchronized timer access. Protect the timer and a closed flag with a mutex shared by the callback and Close, and make the callback's send non-blocking so it can never block while holding the lock. Assisted-by: Claude:claude-opus-5-5 --- nmxact/nmp/dispatch.go | 23 ++++++++++++++++++++++- nmxact/nmp/dispatch_test.go | 11 +++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/nmxact/nmp/dispatch.go b/nmxact/nmp/dispatch.go index f8a28bb..724d05b 100644 --- a/nmxact/nmp/dispatch.go +++ b/nmxact/nmp/dispatch.go @@ -35,6 +35,10 @@ type Listener struct { ErrChan chan error tmoChan chan time.Time timer *time.Timer + + // Protects tmoChan and timer against the timer callback. + mtx sync.Mutex + closed bool } func NewListener() *Listener { @@ -46,14 +50,31 @@ func NewListener() *Listener { } func (nl *Listener) AfterTimeout(tmo time.Duration) <-chan time.Time { + nl.mtx.Lock() + defer nl.mtx.Unlock() + fn := func() { - nl.tmoChan <- time.Now() + nl.mtx.Lock() + defer nl.mtx.Unlock() + + // The listener may have been closed after the timer fired. Never + // block here; a pending timeout is as good as a new one. + if !nl.closed { + select { + case nl.tmoChan <- time.Now(): + default: + } + } } nl.timer = time.AfterFunc(tmo, fn) return nl.tmoChan } func (nl *Listener) Close() { + nl.mtx.Lock() + defer nl.mtx.Unlock() + + nl.closed = true if nl.timer != nil { nl.timer.Stop() } diff --git a/nmxact/nmp/dispatch_test.go b/nmxact/nmp/dispatch_test.go index 1fb39b8..497b536 100644 --- a/nmxact/nmp/dispatch_test.go +++ b/nmxact/nmp/dispatch_test.go @@ -366,3 +366,14 @@ func TestDispatcherDuplicateRspDoesNotBlock(t *testing.T) { t.Fatalf("RemoveListener blocked") } } + +// The timer callback may run concurrently with Close. It must neither +// race with Close nor panic with "send on closed channel". The race is +// timing dependent, so try many times. +func TestListenerTimeoutRacesClose(t *testing.T) { + for i := 0; i < 2000; i++ { + nl := NewListener() + nl.AfterTimeout(0) + nl.Close() + } +}
