Skip to content

Commit

Permalink
morph: Fix subscription deadlock
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
carpawell committed Jan 23, 2024
1 parent df2cc08 commit 7716f9b
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions pkg/morph/client/notifications.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -237,11 +237,11 @@ func (c *Client) routeNotifications() {
routeloop:
for {
var connLost bool
c.subs.RLock()
c.switchLock.RLock()

Check warning on line 240 in pkg/morph/client/notifications.go

View check run for this annotation

Codecov / codecov/patch

pkg/morph/client/notifications.go#L240

Added line #L240 was not covered by tests
notifCh := c.subs.curNotifyChan
blCh := c.subs.curBlockChan
notaryCh := c.subs.curNotaryChan
c.subs.RUnlock()
c.switchLock.RUnlock()

Check warning on line 244 in pkg/morph/client/notifications.go

View check run for this annotation

Codecov / codecov/patch

pkg/morph/client/notifications.go#L244

Added line #L244 was not covered by tests
select {
case <-c.closeChan:
break routeloop
Expand Down

0 comments on commit 7716f9b

Please sign in to comment.