Skip to content

Commit 1c948a5

Browse files
committed
Order ChannelManager lock acquisition
Since last_block_hash is read while updating channels and written when blocks are connected and disconnected, its lock should be held for the duration of these operations. Further, since the channel_state lock is already held during these operations, an ordering of lock acquisition should be maintained to enforce consistency. Ordering lock acquisition such that channel_state is held for the duration of last_block_hash gives a consistent ordering.
1 parent 81131a0 commit 1c948a5

File tree

1 file changed

+12
-5
lines changed

1 file changed

+12
-5
lines changed

lightning/src/ln/channelmanager.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3186,6 +3186,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
31863186
pub fn block_connected(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
31873187
let block_hash = header.block_hash();
31883188
log_trace!(self.logger, "Block {} at height {} connected", block_hash, height);
3189+
31893190
let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
31903191
let mut failed_channels = Vec::new();
31913192
let mut timed_out_htlcs = Vec::new();
@@ -3270,16 +3271,19 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
32703271
});
32713272
!htlcs.is_empty() // Only retain this entry if htlcs has at least one entry.
32723273
});
3274+
3275+
*self.last_block_hash.lock().unwrap() = block_hash;
3276+
self.latest_block_height.store(height as usize, Ordering::Release);
32733277
}
3278+
32743279
for failure in failed_channels.drain(..) {
32753280
self.finish_force_close_channel(failure);
32763281
}
32773282

32783283
for (source, payment_hash, reason) in timed_out_htlcs.drain(..) {
32793284
self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), source, &payment_hash, reason);
32803285
}
3281-
self.latest_block_height.store(height as usize, Ordering::Release);
3282-
*self.last_block_hash.try_lock().expect("block_(dis)connected must not be called in parallel") = block_hash;
3286+
32833287
loop {
32843288
// Update last_node_announcement_serial to be the max of its current value and the
32853289
// block timestamp. This should keep us close to the current time without relying on
@@ -3322,12 +3326,14 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
33223326
true
33233327
}
33243328
});
3329+
3330+
*self.last_block_hash.lock().unwrap() = header.block_hash();
3331+
self.latest_block_height.fetch_sub(1, Ordering::AcqRel);
33253332
}
3333+
33263334
for failure in failed_channels.drain(..) {
33273335
self.finish_force_close_channel(failure);
33283336
}
3329-
self.latest_block_height.fetch_sub(1, Ordering::AcqRel);
3330-
*self.last_block_hash.try_lock().expect("block_(dis)connected must not be called in parallel") = header.block_hash();
33313337
}
33323338

33333339
/// Blocks until ChannelManager needs to be persisted or a timeout is reached. It returns a bool
@@ -3881,11 +3887,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
38813887
writer.write_all(&[SERIALIZATION_VERSION; 1])?;
38823888
writer.write_all(&[MIN_SERIALIZATION_VERSION; 1])?;
38833889

3890+
let channel_state = self.channel_state.lock().unwrap();
3891+
38843892
self.genesis_hash.write(writer)?;
38853893
(self.latest_block_height.load(Ordering::Acquire) as u32).write(writer)?;
38863894
self.last_block_hash.lock().unwrap().write(writer)?;
38873895

3888-
let channel_state = self.channel_state.lock().unwrap();
38893896
let mut unfunded_channels = 0;
38903897
for (_, channel) in channel_state.by_id.iter() {
38913898
if !channel.is_funding_initiated() {

0 commit comments

Comments
 (0)