From 7716f9bed56c502a617b8dcf52b0f68c02f1b8e2 Mon Sep 17 00:00:00 2001 From: Pavel Karpy Date: Tue, 23 Jan 2024 15:27:38 +0300 Subject: [PATCH] morph: Fix subscription deadlock Scenario: 0. at least one subscription has been performed 1. another subscription is being done 2. a notification from one of the `0.` point's subs is received If `2.` happens b/w `0.` and `1.` a deadlock appears since the notification routing process is locked on the subscription lock while the subscription lock cannot be unlocked since the subscription RPC cannot be done before the just-arrived notification is handled (read from the neo-go subscription channel). `switchLock` does the same thing to the `routeNotifications`: it ensures that no routine is doing/will be doing changes with the subscription channels, even though `subs`'s lock was created for this purpose initially. Relates #2559. Signed-off-by: Pavel Karpy --- pkg/morph/client/notifications.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/morph/client/notifications.go b/pkg/morph/client/notifications.go index 0c26eca2f2b..cf755db2e80 100644 --- a/pkg/morph/client/notifications.go +++ b/pkg/morph/client/notifications.go @@ -206,8 +206,6 @@ func (c *Client) Notifications() (<-chan *state.ContainedNotificationEvent, <-ch } type subscriptions struct { - sync.RWMutex - // notification consumers (Client sends // notifications to these channels) notifyChan chan *state.ContainedNotificationEvent @@ -220,6 +218,8 @@ type subscriptions struct { curBlockChan chan *block.Block curNotaryChan chan *result.NotaryRequestEvent + sync.RWMutex // for subscription fields only + // cached subscription information subscribedEvents map[util.Uint160]struct{} subscribedToAllNotaryEvents bool @@ -237,11 +237,11 @@ func (c *Client) routeNotifications() { routeloop: for { var connLost bool - c.subs.RLock() + c.switchLock.RLock() notifCh := c.subs.curNotifyChan blCh := c.subs.curBlockChan notaryCh := c.subs.curNotaryChan - c.subs.RUnlock() + c.switchLock.RUnlock() select { case <-c.closeChan: break routeloop