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 3d69edbbbc40cd69ed66cd4dcd992831b8086785 Author: Szymon Janc <[email protected]> AuthorDate: Thu Oct 22 00:31:12 2026 +0200 nmxact/nmble: Keep ErrorAll error when closing listener Dispatcher.ErrorAll() queues the error on each listener's ErrChan and then calls Listener.Close(). Since 39e5357 Close() drains ErrChan after closing it, which throws away the error that was just queued unless the owner happened to be blocked on the channel at that very moment. The owner then only sees a closed ErrChan, i.e. a nil error. BLE procedures in ble_act.go return that nil error as success (e.g. connect() returning connection handle 0), and the connection monitor in conn.go skips its shutdown with the transport error. Close ErrChan without draining it so a pending error is still delivered before the closed channel is observed. Assisted-by: Claude:claude-opus-5-5 --- nmxact/nmble/dispatch_test.go | 42 ++++++++++++++++++++++++++++++++++++------ nmxact/nmble/listen.go | 7 ++----- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/nmxact/nmble/dispatch_test.go b/nmxact/nmble/dispatch_test.go index 2cb814a..d51403d 100644 --- a/nmxact/nmble/dispatch_test.go +++ b/nmxact/nmble/dispatch_test.go @@ -352,14 +352,13 @@ func TestDispatcherErrorAll(t *testing.T) { d.ErrorAll(want) for i, l := range lners { - // The error is delivered before the listener is closed. select { - case err, ok := <-l.ErrChan: - if ok && err != want { + case err := <-l.ErrChan: + if err != want { t.Errorf("listener %d: got %v, want %v", i, err, want) } case <-time.After(testTimeout): - t.Fatalf("listener %d: ErrChan neither written nor closed", i) + t.Fatalf("listener %d: no error delivered", i) } if _, ok := <-l.MsgChan; ok { t.Errorf("listener %d: MsgChan not closed", i) @@ -520,8 +519,16 @@ func TestListenerCloseDrains(t *testing.T) { l.Close() - if !isClosedMsg(l.MsgChan) || !isClosedErr(l.ErrChan) { - t.Errorf("channels not drained and closed") + if !isClosedMsg(l.MsgChan) { + t.Errorf("MsgChan not drained and closed") + } + + // A pending error is kept for the owner. + if err, ok := <-l.ErrChan; !ok || err == nil { + t.Errorf("pending error lost by Close") + } + if !isClosedErr(l.ErrChan) { + t.Errorf("ErrChan not closed") } if l.timer.Stop() { t.Errorf("timer still active after Close") @@ -554,3 +561,26 @@ func isClosedTmo(c chan time.Time) bool { return false } } + +// ErrorAll must deliver the error even if the listener owner is not +// blocked on ErrChan at that moment (e.g. it is busy handling a message). +// Previously Close() drained the error that ErrorAll had just queued, so +// the owner only saw a closed channel and a nil error. +func TestDispatcherErrorAllDeliversError(t *testing.T) { + d := NewDispatcher() + l := addTestListener(t, d, SeqKey(1)) + + want := errors.New("xport stopped") + d.ErrorAll(want) + + err, ok := <-l.ErrChan + if !ok { + t.Fatalf("ErrChan closed without delivering the error") + } + if err != want { + t.Fatalf("got %v, want %v", err, want) + } + if _, ok := <-l.ErrChan; ok { + t.Errorf("ErrChan not closed after the error") + } +} diff --git a/nmxact/nmble/listen.go b/nmxact/nmble/listen.go index ed0849c..9954661 100644 --- a/nmxact/nmble/listen.go +++ b/nmxact/nmble/listen.go @@ -98,12 +98,9 @@ func (bl *Listener) Close() { } } + // Don't drain ErrChan. Dispatcher.ErrorAll() queues an error right + // before closing the listener, and the owner must still receive it. close(bl.ErrChan) - for { - if _, ok := <-bl.ErrChan; !ok { - break - } - } close(bl.TmoChan) for {
