From 9165d8a5b0e67940be573329706d58c9bc882f1e Mon Sep 17 00:00:00 2001 From: Hank Donnay Date: Mon, 26 Aug 2024 11:53:26 -0500 Subject: [PATCH] stomp: fix Unsubscribe race I think this is OK because the only point that wakes the `sync.Cond` is _after_ closing the channel and updating the `status` member. There should be no way to close the channel _before_ updating the status. If there were, this code would be racy again. Signed-off-by: Hank Donnay --- subscription.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/subscription.go b/subscription.go index d1083e5..8ca54af 100644 --- a/subscription.go +++ b/subscription.go @@ -102,8 +102,12 @@ func (s *Subscription) Unsubscribe(opts ...func(*frame.Frame) error) error { for atomic.LoadInt32(&s.state) != subStateClosed { err = waitWithTimeout(s.closeCond, s.unsubscribeReceiptTimeout) if err != nil && errors.Is(err, &ErrUnsubscribeReceiptTimeout) { - msg := s.subscriptionErrorMessage("channel unsubscribe receipt timeout") - s.C <- msg + // The [closeCond.Broadcast] can race with the timeout, so make sure + // the channel is still available. + if atomic.LoadInt32(&s.state) != subStateClosed { + msg := s.subscriptionErrorMessage("channel unsubscribe receipt timeout") + s.C <- msg + } return err } }