chain: prevent deadlock while notifying RescanFinished for NeutrinoClient #624
Conversation
| // Release the lock while dispatching the notification since | ||
| // it's possible for the notificationHandler to be waiting to | ||
| // acquire it before receiving the notification. | ||
| s.clientMtx.Unlock() |
There was a problem hiding this comment.
Nice, I think this is an ok solution for now, this should fix the issue I ran into. Longer term we should move to a more granular set of mutexes (or get rid of the mutexes all together), but short term this is fine 👍
chain/neutrino.go
Outdated
| case <-rescanQuit: | ||
| // We'll need to re-acquire the lock in order for the | ||
| // deferred release call to not panic. | ||
| s.clientMtx.Lock() |
There was a problem hiding this comment.
Could we instead get rid of the defer and unlock manually on each return case?
There was a problem hiding this comment.
@cfromknecht seems to prefer the defer approach, though that might've been because there were some fields that were being used without the lock in the prior diff.
There was a problem hiding this comment.
Yeah I’m fine with not having a defer, originally thought it’d make a smaller diff but seems not :P
chain/neutrino.go
Outdated
| return nil | ||
| } | ||
| s.clientMtx.Lock() |
There was a problem hiding this comment.
I noticed that we are not returning the error on line 391
No description provided.