Skip to content

Commit f008fd2

Browse files
committed
fix: Fix reentrancy bug in read_event()
Per the external documentation, there is an invariant that read_event() will not call back into the SocketDescriptor, but the actual code flushed the outbound queue. Fix the bug and use the type system to guarantee this behavior in the future. By changing the function parameters to use `impl PayloadQueuer` the function will only have access to functions on that interface throughout the execution, but still be able to pass the OutboundQueue object since it satisfies the trait bounds. Functions such as enqueue_message() also take a `impl PayloadQueuer` so they can be called from that context, but functions that need the SocketDescriptorFlusher interface will fail at compile-time and point to the issue before any tests need to be run. This also fixes up tests and the tokio implementation that did not implement the proper behavior and relied on read_event() calling send_data().
1 parent 3a503d4 commit f008fd2

File tree

3 files changed

+177
-94
lines changed

3 files changed

+177
-94
lines changed

lightning-net-tokio/src/lib.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -189,17 +189,20 @@ impl Connection {
189189
Ok(len) => {
190190
prepare_read_write_call!();
191191
let read_res = peer_manager.read_event(&mut our_descriptor, &buf[0..len]);
192-
let mut us_lock = us.lock().unwrap();
193-
match read_res {
194-
Ok(pause_read) => {
195-
if pause_read {
196-
us_lock.read_paused = true;
197-
}
198-
Self::event_trigger(&mut us_lock);
199-
},
200-
Err(e) => shutdown_socket!(e, Disconnect::CloseConnection),
192+
{
193+
let mut us_lock = us.lock().unwrap();
194+
match read_res {
195+
Ok(pause_read) => {
196+
if pause_read {
197+
us_lock.read_paused = true;
198+
}
199+
Self::event_trigger(&mut us_lock);
200+
},
201+
Err(e) => shutdown_socket!(e, Disconnect::CloseConnection),
202+
}
203+
us_lock.block_disconnect_socket = false;
201204
}
202-
us_lock.block_disconnect_socket = false;
205+
peer_manager.process_events();
203206
},
204207
Err(e) => shutdown_socket!(e, Disconnect::PeerDisconnected),
205208
},

0 commit comments

Comments
 (0)