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 d033c8e6d0b8112a2dba7214f9a50d29492ff180 Author: Szymon Janc <[email protected]> AuthorDate: Thu Oct 22 00:21:57 2026 +0200 nmxact/nmp: Fix RX deadlock on duplicate NMP response DispatchRsp sent the response to the listener's 1-deep RspChan while holding the dispatcher mutex. If a device sent two responses with the same sequence number (e.g. a retransmission) before the client consumed the first one, the second send blocked forever with the lock held. The client's subsequent RemoveListener then blocked on the same lock, so the receive path and the whole session hung. Only one response per sequence number is expected, so drop extra responses instead of blocking. Assisted-by: Claude:claude-opus-5-5 --- nmxact/nmp/dispatch.go | 9 ++++++++- nmxact/nmp/dispatch_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/nmxact/nmp/dispatch.go b/nmxact/nmp/dispatch.go index 4bf57b2..f8a28bb 100644 --- a/nmxact/nmp/dispatch.go +++ b/nmxact/nmp/dispatch.go @@ -138,7 +138,14 @@ func (d *Dispatcher) DispatchRsp(r NmpRsp) bool { return false } - nl.RspChan <- r + // Only one response per sequence number is expected. Drop duplicates + // rather than blocking the receive path while holding the lock. + select { + case nl.RspChan <- r: + default: + log.Debugf("Dropping duplicate NMP response; seq=%d", r.Hdr().Seq) + return false + } return true } diff --git a/nmxact/nmp/dispatch_test.go b/nmxact/nmp/dispatch_test.go index 596bf4d..1fb39b8 100644 --- a/nmxact/nmp/dispatch_test.go +++ b/nmxact/nmp/dispatch_test.go @@ -323,3 +323,46 @@ func TestListenerCloseStopsTimer(t *testing.T) { t.Errorf("timer still active after Close") } } + +// A device may send more than one response with the same sequence number +// (e.g. a retransmission). The RX path must not block on the extra +// responses, otherwise the listener can never be removed. +func TestDispatcherDuplicateRspDoesNotBlock(t *testing.T) { + d := NewDispatcher(0) + nl, err := d.AddListener(3) + if err != nil { + t.Fatal(err) + } + + b := encodeTestEchoRsp(t, NMP_OP_WRITE_RSP, 3, "dup") + + done := make(chan struct{}) + go func() { + defer close(done) + for i := 0; i < 3; i++ { + d.Dispatch(b) + } + }() + + select { + case <-done: + case <-time.After(testTimeout): + t.Fatalf("Dispatch of duplicate response blocked") + } + + if echo := recvRsp(t, nl).(*EchoRsp); echo.Payload != "dup" { + t.Errorf("payload = %q", echo.Payload) + } + + removed := make(chan struct{}) + go func() { + d.RemoveListener(3) + close(removed) + }() + + select { + case <-removed: + case <-time.After(testTimeout): + t.Fatalf("RemoveListener blocked") + } +}
