Skip to content

Commit 389c4ad

Browse files
jkczyzjkczyz
committed
Change Mutex to RwLock in ChainMonitor
Now that ChannelMonitor uses an internal Mutex to support interior mutability, ChainMonitor can use a RwLock to manage its ChannelMonitor map. This allows parallelization of update_channel operations since an exclusive lock only needs to be held when adding to the map in watch_channel.
1 parent 8e558fe commit 389c4ad

File tree

5 files changed

+29
-29
lines changed

5 files changed

+29
-29
lines changed

‎lightning/src/chain/chainmonitor.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ use util::events;
4343
use util::events::Event;
4444

4545
use std::collections::{HashMap, hash_map};
46-
use std::sync::Mutex;
46+
use std::sync::RwLock;
4747
use std::ops::Deref;
4848

4949
/// An implementation of [`chain::Watch`] for monitoring channels.
@@ -64,7 +64,7 @@ pub struct ChainMonitor<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: De
6464
P::Target: channelmonitor::Persist<ChannelSigner>,
6565
{
6666
/// The monitors
67-
pub monitors: Mutex<HashMap<OutPoint, ChannelMonitor<ChannelSigner>>>,
67+
pub monitors: RwLock<HashMap<OutPoint, ChannelMonitor<ChannelSigner>>>,
6868
chain_source: Option<C>,
6969
broadcaster: T,
7070
logger: L,
@@ -93,8 +93,8 @@ where C::Target: chain::Filter,
9393
/// [`chain::Watch::release_pending_monitor_events`]: ../trait.Watch.html#tymethod.release_pending_monitor_events
9494
/// [`chain::Filter`]: ../trait.Filter.html
9595
pub fn block_connected(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
96-
let mut monitors = self.monitors.lock().unwrap();
97-
for monitor in monitors.values_mut() {
96+
let monitors = self.monitors.read().unwrap();
97+
for monitor in monitors.values() {
9898
let mut txn_outputs = monitor.block_connected(header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
9999

100100
if let Some(ref chain_source) = self.chain_source {
@@ -113,8 +113,8 @@ where C::Target: chain::Filter,
113113
///
114114
/// [`ChannelMonitor::block_disconnected`]: ../channelmonitor/struct.ChannelMonitor.html#method.block_disconnected
115115
pub fn block_disconnected(&self, header: &BlockHeader, disconnected_height: u32) {
116-
let mut monitors = self.monitors.lock().unwrap();
117-
for monitor in monitors.values_mut() {
116+
let monitors = self.monitors.read().unwrap();
117+
for monitor in monitors.values() {
118118
monitor.block_disconnected(header, disconnected_height, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
119119
}
120120
}
@@ -130,7 +130,7 @@ where C::Target: chain::Filter,
130130
/// [`chain::Filter`]: ../trait.Filter.html
131131
pub fn new(chain_source: Option<C>, broadcaster: T, logger: L, feeest: F, persister: P) -> Self {
132132
Self {
133-
monitors: Mutex::new(HashMap::new()),
133+
monitors: RwLock::new(HashMap::new()),
134134
chain_source,
135135
broadcaster,
136136
logger,
@@ -177,7 +177,7 @@ where C::Target: chain::Filter,
177177
///
178178
/// [`chain::Filter`]: ../trait.Filter.html
179179
fn watch_channel(&self, funding_outpoint: OutPoint, monitor: ChannelMonitor<ChannelSigner>) -> Result<(), ChannelMonitorUpdateErr> {
180-
let mut monitors = self.monitors.lock().unwrap();
180+
let mut monitors = self.monitors.write().unwrap();
181181
let entry = match monitors.entry(funding_outpoint) {
182182
hash_map::Entry::Occupied(_) => {
183183
log_error!(self.logger, "Failed to add new channel data: channel monitor for given outpoint is already present");
@@ -209,8 +209,8 @@ where C::Target: chain::Filter,
209209
/// `ChainMonitor` monitors lock.
210210
fn update_channel(&self, funding_txo: OutPoint, update: ChannelMonitorUpdate) -> Result<(), ChannelMonitorUpdateErr> {
211211
// Update the monitor that watches the channel referred to by the given outpoint.
212-
let mut monitors = self.monitors.lock().unwrap();
213-
match monitors.get_mut(&funding_txo) {
212+
let monitors = self.monitors.read().unwrap();
213+
match monitors.get(&funding_txo) {
214214
None => {
215215
log_error!(self.logger, "Failed to update channel monitor: no such monitor registered");
216216

@@ -245,7 +245,7 @@ where C::Target: chain::Filter,
245245

246246
fn release_pending_monitor_events(&self) -> Vec<MonitorEvent> {
247247
let mut pending_monitor_events = Vec::new();
248-
for monitor in self.monitors.lock().unwrap().values_mut() {
248+
for monitor in self.monitors.read().unwrap().values() {
249249
pending_monitor_events.append(&mut monitor.get_and_clear_pending_monitor_events());
250250
}
251251
pending_monitor_events
@@ -261,7 +261,7 @@ impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> even
261261
{
262262
fn get_and_clear_pending_events(&self) -> Vec<Event> {
263263
let mut pending_events = Vec::new();
264-
for monitor in self.monitors.lock().unwrap().values_mut() {
264+
for monitor in self.monitors.read().unwrap().values() {
265265
pending_events.append(&mut monitor.get_and_clear_pending_events());
266266
}
267267
pending_events

‎lightning/src/ln/chanmon_update_fail_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ fn test_monitor_and_persister_update_fail() {
102102
let logger = test_utils::TestLogger::with_id(format!("node {}", 0));
103103
let persister = test_utils::TestPersister::new();
104104
let chain_mon = {
105-
let monitors = nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap();
105+
let monitors = nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap();
106106
let monitor = monitors.get(&outpoint).unwrap();
107107
let mut w = test_utils::TestVecWriter(Vec::new());
108108
monitor.write(&mut w).unwrap();

‎lightning/src/ln/functional_test_utils.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
168168
let feeest = test_utils::TestFeeEstimator { sat_per_kw: 253 };
169169
let mut deserialized_monitors = Vec::new();
170170
{
171-
let old_monitors = self.chain_monitor.chain_monitor.monitors.lock().unwrap();
171+
let old_monitors = self.chain_monitor.chain_monitor.monitors.read().unwrap();
172172
for (_, old_monitor) in old_monitors.iter() {
173173
let mut w = test_utils::TestVecWriter(Vec::new());
174174
old_monitor.write(&mut w).unwrap();
@@ -305,9 +305,9 @@ macro_rules! get_feerate {
305305
macro_rules! get_local_commitment_txn {
306306
($node: expr, $channel_id: expr) => {
307307
{
308-
let mut monitors = $node.chain_monitor.chain_monitor.monitors.lock().unwrap();
308+
let monitors = $node.chain_monitor.chain_monitor.monitors.read().unwrap();
309309
let mut commitment_txn = None;
310-
for (funding_txo, monitor) in monitors.iter_mut() {
310+
for (funding_txo, monitor) in monitors.iter() {
311311
if funding_txo.to_channel_id() == $channel_id {
312312
commitment_txn = Some(monitor.unsafe_get_latest_holder_commitment_txn(&$node.logger));
313313
break;

‎lightning/src/ln/functional_tests.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3517,8 +3517,8 @@ fn test_force_close_fail_back() {
35173517

35183518
// Now check that if we add the preimage to ChannelMonitor it broadcasts our HTLC-Success..
35193519
{
3520-
let mut monitors = nodes[2].chain_monitor.chain_monitor.monitors.lock().unwrap();
3521-
monitors.get_mut(&OutPoint{ txid: Txid::from_slice(&payment_event.commitment_msg.channel_id[..]).unwrap(), index: 0 }).unwrap()
3520+
let mut monitors = nodes[2].chain_monitor.chain_monitor.monitors.read().unwrap();
3521+
monitors.get(&OutPoint{ txid: Txid::from_slice(&payment_event.commitment_msg.channel_id[..]).unwrap(), index: 0 }).unwrap()
35223522
.provide_payment_preimage(&our_payment_hash, &our_payment_preimage, &node_cfgs[2].tx_broadcaster, &node_cfgs[2].fee_estimator, &&logger);
35233523
}
35243524
connect_block(&nodes[2], &block, 1);
@@ -4314,7 +4314,7 @@ fn test_no_txn_manager_serialize_deserialize() {
43144314

43154315
let nodes_0_serialized = nodes[0].node.encode();
43164316
let mut chan_0_monitor_serialized = test_utils::TestVecWriter(Vec::new());
4317-
nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap();
4317+
nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap();
43184318

43194319
logger = test_utils::TestLogger::new();
43204320
fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
@@ -4423,7 +4423,7 @@ fn test_manager_serialize_deserialize_events() {
44234423
// Start the de/seriailization process mid-channel creation to check that the channel manager will hold onto events that are serialized
44244424
let nodes_0_serialized = nodes[0].node.encode();
44254425
let mut chan_0_monitor_serialized = test_utils::TestVecWriter(Vec::new());
4426-
nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap();
4426+
nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap();
44274427

44284428
fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
44294429
logger = test_utils::TestLogger::new();
@@ -4515,7 +4515,7 @@ fn test_simple_manager_serialize_deserialize() {
45154515

45164516
let nodes_0_serialized = nodes[0].node.encode();
45174517
let mut chan_0_monitor_serialized = test_utils::TestVecWriter(Vec::new());
4518-
nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap();
4518+
nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap();
45194519

45204520
logger = test_utils::TestLogger::new();
45214521
fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
@@ -4572,7 +4572,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
45724572
let (_, _, channel_id, funding_tx) = create_announced_chan_between_nodes(&nodes, 0, 3, InitFeatures::known(), InitFeatures::known());
45734573

45744574
let mut node_0_stale_monitors_serialized = Vec::new();
4575-
for monitor in nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter() {
4575+
for monitor in nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap().iter() {
45764576
let mut writer = test_utils::TestVecWriter(Vec::new());
45774577
monitor.1.write(&mut writer).unwrap();
45784578
node_0_stale_monitors_serialized.push(writer.0);
@@ -4591,7 +4591,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
45914591
// Now the ChannelMonitor (which is now out-of-sync with ChannelManager for channel w/
45924592
// nodes[3])
45934593
let mut node_0_monitors_serialized = Vec::new();
4594-
for monitor in nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter() {
4594+
for monitor in nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap().iter() {
45954595
let mut writer = test_utils::TestVecWriter(Vec::new());
45964596
monitor.1.write(&mut writer).unwrap();
45974597
node_0_monitors_serialized.push(writer.0);
@@ -7479,7 +7479,7 @@ fn test_data_loss_protect() {
74797479
// Cache node A state before any channel update
74807480
let previous_node_state = nodes[0].node.encode();
74817481
let mut previous_chain_monitor_state = test_utils::TestVecWriter(Vec::new());
7482-
nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter().next().unwrap().1.write(&mut previous_chain_monitor_state).unwrap();
7482+
nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap().iter().next().unwrap().1.write(&mut previous_chain_monitor_state).unwrap();
74837483

74847484
send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000, 8_000_000);
74857485
send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000, 8_000_000);
@@ -8226,7 +8226,7 @@ fn test_bump_txn_sanitize_tracking_maps() {
82268226
connect_block(&nodes[0], &Block { header: header_130, txdata: penalty_txn }, 130);
82278227
connect_blocks(&nodes[0], 5, 130, false, header_130.block_hash());
82288228
{
8229-
let monitors = nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap();
8229+
let monitors = nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap();
82308230
if let Some(monitor) = monitors.get(&OutPoint { txid: chan.3.txid(), index: 0 }) {
82318231
assert!(monitor.inner.lock().unwrap().onchain_tx_handler.pending_claim_requests.is_empty());
82328232
assert!(monitor.inner.lock().unwrap().onchain_tx_handler.claimable_outpoints.is_empty());
@@ -8360,7 +8360,7 @@ fn test_update_err_monitor_lockdown() {
83608360
let logger = test_utils::TestLogger::with_id(format!("node {}", 0));
83618361
let persister = test_utils::TestPersister::new();
83628362
let watchtower = {
8363-
let monitors = nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap();
8363+
let monitors = nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap();
83648364
let monitor = monitors.get(&outpoint).unwrap();
83658365
let mut w = test_utils::TestVecWriter(Vec::new());
83668366
monitor.write(&mut w).unwrap();
@@ -8419,7 +8419,7 @@ fn test_concurrent_monitor_claim() {
84198419
let logger = test_utils::TestLogger::with_id(format!("node {}", "Alice"));
84208420
let persister = test_utils::TestPersister::new();
84218421
let watchtower_alice = {
8422-
let monitors = nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap();
8422+
let monitors = nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap();
84238423
let monitor = monitors.get(&outpoint).unwrap();
84248424
let mut w = test_utils::TestVecWriter(Vec::new());
84258425
monitor.write(&mut w).unwrap();
@@ -8445,7 +8445,7 @@ fn test_concurrent_monitor_claim() {
84458445
let logger = test_utils::TestLogger::with_id(format!("node {}", "Bob"));
84468446
let persister = test_utils::TestPersister::new();
84478447
let watchtower_bob = {
8448-
let monitors = nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap();
8448+
let monitors = nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap();
84498449
let monitor = monitors.get(&outpoint).unwrap();
84508450
let mut w = test_utils::TestVecWriter(Vec::new());
84518451
monitor.write(&mut w).unwrap();

‎lightning/src/util/test_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ impl<'a> chain::Watch<EnforcingSigner> for TestChainMonitor<'a> {
133133
let update_res = self.chain_monitor.update_channel(funding_txo, update);
134134
// At every point where we get a monitor update, we should be able to send a useful monitor
135135
// to a watchtower and disk...
136-
let monitors = self.chain_monitor.monitors.lock().unwrap();
136+
let monitors = self.chain_monitor.monitors.read().unwrap();
137137
let monitor = monitors.get(&funding_txo).unwrap();
138138
w.0.clear();
139139
monitor.write(&mut w).unwrap();

0 commit comments

Comments
 (0)