Expect callers to hold read locks before `channel_monitor_updated`
authorMatt Corallo <git@bluematt.me>
Fri, 3 Feb 2023 00:46:50 +0000 (00:46 +0000)
committerMatt Corallo <git@bluematt.me>
Tue, 28 Feb 2023 01:06:35 +0000 (01:06 +0000)
commit38374dde42991ec1829ac07033a313e883c4242e
treea57eb726bfed51325022e1b771bdaed538d06154
parent1d4bc1e0fb491a255dc7575ef77703bec073bbc8
Expect callers to hold read locks before `channel_monitor_updated`

Our existing lockorder tests assume that a read lock on a thread
that is already holding the same read lock is totally fine. This
isn't at all true. The `std` `RwLock` behavior is
platform-dependent - on most platforms readers can starve writers
as readers will never block for a pending writer. However, on
platforms where this is not the case, one thread trying to take a
write lock may deadlock with another thread that both already has,
and is attempting to take again, a read lock.

Worse, our in-tree `FairRwLock` exhibits this behavior explicitly
on all platforms to avoid the starvation issue.

Sadly, a user ended up hitting this deadlock in production in the
form of a call to `get_and_clear_pending_msg_events` which holds
the `ChannelManager::total_consistency_lock` before calling
`process_pending_monitor_events` and eventually
`channel_monitor_updated`, which tries to take the same read lock
again.

Luckily, the fix is trivial, simply remove the redundand read lock
in `channel_monitor_updated`.

Fixes #2000
lightning/src/ln/channelmanager.rs