From: Matt Corallo <git@bluematt.me>
Date: Wed, 16 Jun 2021 18:57:21 +0000 (+0000)
Subject: Fix bogus reentrancy from read_event -> do_attempt_write_data
X-Git-Tag: v0.0.99~19^2~6
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=a6463ec6cd8f8604e1e1b6e3300b144c5a1b6e4b;p=rust-lightning

Fix bogus reentrancy from read_event -> do_attempt_write_data

`Julian Knutsen <julianknutsen@users.noreply.github.com>` 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 #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.
---

diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs
index 2003efc8f..cc15d26d5 100644
--- a/lightning/src/ln/peer_handler.rs
+++ b/lightning/src/ln/peer_handler.rs
@@ -807,8 +807,6 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref> PeerManager<D
 						}
 					}
 
-					self.do_attempt_write_data(peer_descriptor, peer);
-
 					peer.pending_outbound_buffer.len() > 10 // pause_read
 				}
 			};
@@ -1467,7 +1465,9 @@ mod tests {
 		let initial_data = peer_b.new_outbound_connection(a_id, fd_b.clone()).unwrap();
 		peer_a.new_inbound_connection(fd_a.clone()).unwrap();
 		assert_eq!(peer_a.read_event(&mut fd_a, &initial_data).unwrap(), false);
+		peer_a.process_events();
 		assert_eq!(peer_b.read_event(&mut fd_b, &fd_a.outbound_data.lock().unwrap().split_off(0)).unwrap(), false);
+		peer_b.process_events();
 		assert_eq!(peer_a.read_event(&mut fd_a, &fd_b.outbound_data.lock().unwrap().split_off(0)).unwrap(), false);
 		(fd_a.clone(), fd_b.clone())
 	}
@@ -1506,10 +1506,12 @@ mod tests {
 
 		// peers[0] awaiting_pong is set to true, but the Peer is still connected
 		peers[0].timer_tick_occurred();
+		peers[0].process_events();
 		assert_eq!(peers[0].peers.lock().unwrap().peers.len(), 1);
 
 		// Since timer_tick_occurred() is called again when awaiting_pong is true, all Peers are disconnected
 		peers[0].timer_tick_occurred();
+		peers[0].process_events();
 		assert_eq!(peers[0].peers.lock().unwrap().peers.len(), 0);
 	}
 
@@ -1530,7 +1532,9 @@ mod tests {
 		let (mut fd_a, mut fd_b) = establish_connection(&peers[0], &peers[1]);
 
 		// Make each peer to read the messages that the other peer just wrote to them.
+		peers[0].process_events();
 		peers[1].read_event(&mut fd_b, &fd_a.outbound_data.lock().unwrap().split_off(0)).unwrap();
+		peers[1].process_events();
 		peers[0].read_event(&mut fd_a, &fd_b.outbound_data.lock().unwrap().split_off(0)).unwrap();
 
 		// Check that each peer has received the expected number of channel updates and channel