invoices: fix deadlock in notification handling

This commit is contained in:
Andras Banki-Horvath 2022-05-31 17:24:08 +02:00
parent 1e0d6ec0ad
commit cc8a1f0d2b
No known key found for this signature in database
GPG Key ID: 80E5375C094198D8

View File

@ -108,6 +108,12 @@ type InvoiceRegistry struct {
// cfg contains the registry's configuration parameters. // cfg contains the registry's configuration parameters.
cfg *RegistryConfig cfg *RegistryConfig
// notificationClientMux locks notificationClients and
// singleNotificationClients. Using a separate mutex for these maps is
// necessary to avoid deadlocks in the registry when processing invoice
// events.
notificationClientMux sync.RWMutex
notificationClients map[uint32]*InvoiceSubscription notificationClients map[uint32]*InvoiceSubscription
// TODO(yy): use map[lntypes.Hash]*SingleInvoiceSubscription for better // TODO(yy): use map[lntypes.Hash]*SingleInvoiceSubscription for better
@ -1584,9 +1590,9 @@ func (i *InvoiceRegistry) SubscribeNotifications(
} }
}() }()
i.Lock() i.notificationClientMux.Lock()
i.notificationClients[client.id] = client i.notificationClients[client.id] = client
i.Unlock() i.notificationClientMux.Unlock()
// Query the database to see if based on the provided addIndex and // Query the database to see if based on the provided addIndex and
// settledIndex we need to deliver any backlog notifications. // settledIndex we need to deliver any backlog notifications.
@ -1660,9 +1666,9 @@ func (i *InvoiceRegistry) SubscribeSingleInvoice(
} }
}() }()
i.Lock() i.notificationClientMux.Lock()
i.singleNotificationClients[client.id] = client i.singleNotificationClients[client.id] = client
i.Unlock() i.notificationClientMux.Unlock()
err := i.deliverSingleBacklogEvents(client) err := i.deliverSingleBacklogEvents(client)
if err != nil { if err != nil {
@ -1739,8 +1745,8 @@ func (i *InvoiceRegistry) HodlUnsubscribeAll(subscriber chan<- interface{}) {
// copySingleClients copies i.SingleInvoiceSubscription inside a lock. This is // copySingleClients copies i.SingleInvoiceSubscription inside a lock. This is
// useful when we need to iterate the map to send notifications. // useful when we need to iterate the map to send notifications.
func (i *InvoiceRegistry) copySingleClients() map[uint32]*SingleInvoiceSubscription { func (i *InvoiceRegistry) copySingleClients() map[uint32]*SingleInvoiceSubscription {
i.RLock() i.notificationClientMux.RLock()
defer i.RUnlock() defer i.notificationClientMux.RUnlock()
clients := make(map[uint32]*SingleInvoiceSubscription) clients := make(map[uint32]*SingleInvoiceSubscription)
for k, v := range i.singleNotificationClients { for k, v := range i.singleNotificationClients {
@ -1752,8 +1758,8 @@ func (i *InvoiceRegistry) copySingleClients() map[uint32]*SingleInvoiceSubscript
// copyClients copies i.notificationClients inside a lock. This is useful when // copyClients copies i.notificationClients inside a lock. This is useful when
// we need to iterate the map to send notifications. // we need to iterate the map to send notifications.
func (i *InvoiceRegistry) copyClients() map[uint32]*InvoiceSubscription { func (i *InvoiceRegistry) copyClients() map[uint32]*InvoiceSubscription {
i.RLock() i.notificationClientMux.RLock()
defer i.RUnlock() defer i.notificationClientMux.RUnlock()
clients := make(map[uint32]*InvoiceSubscription) clients := make(map[uint32]*InvoiceSubscription)
for k, v := range i.notificationClients { for k, v := range i.notificationClients {
@ -1765,8 +1771,8 @@ func (i *InvoiceRegistry) copyClients() map[uint32]*InvoiceSubscription {
// deleteClient removes a client by its ID inside a lock. Noop if the client is // deleteClient removes a client by its ID inside a lock. Noop if the client is
// not found. // not found.
func (i *InvoiceRegistry) deleteClient(clientID uint32) { func (i *InvoiceRegistry) deleteClient(clientID uint32) {
i.Lock() i.notificationClientMux.Lock()
defer i.Unlock() defer i.notificationClientMux.Unlock()
log.Infof("Cancelling invoice subscription for client=%v", clientID) log.Infof("Cancelling invoice subscription for client=%v", clientID)
delete(i.notificationClients, clientID) delete(i.notificationClients, clientID)