Skip to content

Commit 62d4007

Browse files
committed
Fix bogus reentrancy from read_event -> do_attempt_write_data
`Julian Knutsen <[email protected]>` pointed out in a previous discussion that `read_event` can reenter user code despite the documentation stating explicitly that it will not. This was addressed in lightningdevkit#456 by simply codifying the reentrancy, but its somewhat simpler to just drop the `do_attempt_write_data` call. Ideally we could land most of Julian's work, but its still in need of substantial git history cleanup to get it in a reviewable state and this solves the immediate issue.
1 parent 5bf4cad commit 62d4007

File tree

1 file changed

+7
-3
lines changed

1 file changed

+7
-3
lines changed

lightning/src/ln/peer_handler.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -794,7 +794,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref> PeerManager<D
794794
}
795795
};
796796

797-
if let Err(handling_error) = self.handle_message(&mut peers.peers_needing_send, peer, peer_descriptor.clone(), message){
797+
if let Err(handling_error) = self.handle_message(&mut peers.peers_needing_send, peer, peer_descriptor.clone(), message) {
798798
match handling_error {
799799
MessageHandlingError::PeerHandleError(e) => { return Err(e) },
800800
MessageHandlingError::LightningError(e) => {
@@ -808,8 +808,6 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref> PeerManager<D
808808
}
809809
}
810810

811-
self.do_attempt_write_data(peer_descriptor, peer);
812-
813811
peer.pending_outbound_buffer.len() > 10 // pause_read
814812
}
815813
};
@@ -1470,7 +1468,9 @@ mod tests {
14701468
let initial_data = peer_b.new_outbound_connection(a_id, fd_b.clone()).unwrap();
14711469
peer_a.new_inbound_connection(fd_a.clone()).unwrap();
14721470
assert_eq!(peer_a.read_event(&mut fd_a, &initial_data).unwrap(), false);
1471+
peer_a.process_events();
14731472
assert_eq!(peer_b.read_event(&mut fd_b, &fd_a.outbound_data.lock().unwrap().split_off(0)).unwrap(), false);
1473+
peer_b.process_events();
14741474
assert_eq!(peer_a.read_event(&mut fd_a, &fd_b.outbound_data.lock().unwrap().split_off(0)).unwrap(), false);
14751475
(fd_a.clone(), fd_b.clone())
14761476
}
@@ -1509,10 +1509,12 @@ mod tests {
15091509

15101510
// peers[0] awaiting_pong is set to true, but the Peer is still connected
15111511
peers[0].timer_tick_occurred();
1512+
peers[0].process_events();
15121513
assert_eq!(peers[0].peers.lock().unwrap().peers.len(), 1);
15131514

15141515
// Since timer_tick_occurred() is called again when awaiting_pong is true, all Peers are disconnected
15151516
peers[0].timer_tick_occurred();
1517+
peers[0].process_events();
15161518
assert_eq!(peers[0].peers.lock().unwrap().peers.len(), 0);
15171519
}
15181520

@@ -1533,7 +1535,9 @@ mod tests {
15331535
let (mut fd_a, mut fd_b) = establish_connection(&peers[0], &peers[1]);
15341536

15351537
// Make each peer to read the messages that the other peer just wrote to them.
1538+
peers[0].process_events();
15361539
peers[1].read_event(&mut fd_b, &fd_a.outbound_data.lock().unwrap().split_off(0)).unwrap();
1540+
peers[1].process_events();
15371541
peers[0].read_event(&mut fd_a, &fd_b.outbound_data.lock().unwrap().split_off(0)).unwrap();
15381542

15391543
// Check that each peer has received the expected number of channel updates and channel

0 commit comments

Comments
 (0)