Skip to content

Commit b31434d

Browse files
ImplOfAnImplImplOfAnImpl
committed
Put the disconnection and socket write timeouts to BackendTimeoutsConfig; use a smaller socket write timeout in a test
1 parent 2aba261 commit b31434d

File tree

6 files changed

+85
-23
lines changed

6 files changed

+85
-23
lines changed

‎node-lib/src/config_files/p2p.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,8 @@ impl From<P2pConfigFile> for P2pConfig {
191191
.map(|t| Duration::from_secs(t.into()))
192192
.into(),
193193
peer_handshake_timeout: Default::default(),
194+
disconnection_timeout: Default::default(),
195+
socket_write_timeout: Default::default(),
194196
},
195197
protocol_config: Default::default(),
196198
custom_disconnection_reason_for_banning,

‎p2p/src/config.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ make_config_setting!(PingTimeout, Duration, Duration::from_secs(150));
3535
make_config_setting!(MaxClockDiff, Duration, Duration::from_secs(10));
3636
make_config_setting!(SyncStallingTimeout, Duration, Duration::from_secs(25));
3737
make_config_setting!(PeerHandshakeTimeout, Duration, Duration::from_secs(10));
38+
make_config_setting!(DisconnectionTimeout, Duration, Duration::from_secs(10));
39+
make_config_setting!(SoketWriteTimeout, Duration, Duration::from_secs(60));
3840

3941
/// A node type.
4042
#[derive(Debug, Copy, Clone)]
@@ -144,4 +146,10 @@ pub struct BackendTimeoutsConfig {
144146

145147
/// Timeout for initial peer handshake
146148
pub peer_handshake_timeout: PeerHandshakeTimeout,
149+
150+
/// Timeout for disconnection
151+
pub disconnection_timeout: DisconnectionTimeout,
152+
153+
/// Timeout for the socket write call
154+
pub socket_write_timeout: SoketWriteTimeout,
147155
}

‎p2p/src/net/default_backend/peer/mod.rs‎

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ mod handshake_handler;
1717
#[cfg(test)]
1818
mod tests;
1919

20-
use std::{sync::Arc, time::Duration};
20+
use std::sync::Arc;
2121

2222
use tokio::{
2323
sync::mpsc::{self, unbounded_channel},
@@ -232,6 +232,7 @@ where
232232
let (writer_cmd_sender, writer_cmd_receiver) = unbounded_channel();
233233
let (writer_event_sender, mut writer_event_receiver) = unbounded_channel();
234234
let writer_join_handle = spawn_writer(
235+
Arc::clone(&p2p_config),
235236
peer_id,
236237
common_protocol_version.0,
237238
socket_writer,
@@ -332,16 +333,19 @@ where
332333
});
333334
match send_result {
334335
Ok(()) => {
335-
let disconnect_result = tokio::time::timeout(DISCONNECTION_TIMEOUT, async {
336-
match writer_event_receiver.recv().await {
337-
Some(WriterEvent::WriterClosed(result)) => {
338-
log::debug!("Socket writer closing confirmed with result: {result:?}");
339-
},
340-
None => {
341-
log::debug!("Socket writer task already closed when waiting for disconnection");
342-
},
336+
let disconnect_result = tokio::time::timeout(
337+
*p2p_config.backend_timeouts.disconnection_timeout,
338+
async {
339+
match writer_event_receiver.recv().await {
340+
Some(WriterEvent::WriterClosed(result)) => {
341+
log::debug!("Socket writer closing confirmed with result: {result:?}");
342+
},
343+
None => {
344+
log::debug!("Socket writer task already closed when waiting for disconnection");
345+
},
346+
}
343347
}
344-
}).await;
348+
).await;
345349

346350
match disconnect_result {
347351
Ok(()) => {}
@@ -377,9 +381,6 @@ where
377381
}
378382
}
379383

380-
const DISCONNECTION_TIMEOUT: Duration = Duration::from_secs(10);
381-
const SOCKET_WRITE_TIMEOUT: Duration = Duration::from_secs(60);
382-
383384
async fn maybe_send_will_disconnect<S: PeerStream>(
384385
reason: Option<DisconnectionReason>,
385386
peer_protocol_version: ProtocolVersion,
@@ -409,6 +410,7 @@ enum WriterEvent {
409410
}
410411

411412
fn spawn_writer<S: PeerStream + 'static>(
413+
p2p_config: Arc<P2pConfig>,
412414
peer_id: PeerId,
413415
common_protocol_version: SupportedProtocolVersion,
414416
socket_writer: MessageWriter<S, Message>,
@@ -417,8 +419,13 @@ fn spawn_writer<S: PeerStream + 'static>(
417419
) -> JoinHandle<()> {
418420
tokio_spawn_in_current_tracing_span(
419421
async move {
420-
let writer_result =
421-
writer_loop(common_protocol_version, socket_writer, cmd_receiver).await;
422+
let writer_result = writer_loop(
423+
&p2p_config,
424+
common_protocol_version,
425+
socket_writer,
426+
cmd_receiver,
427+
)
428+
.await;
422429

423430
if let Err(_) = event_sender.send(WriterEvent::WriterClosed(writer_result)) {
424431
log::debug!("Peer task already closed");
@@ -429,6 +436,7 @@ fn spawn_writer<S: PeerStream + 'static>(
429436
}
430437

431438
async fn writer_loop<S: PeerStream>(
439+
p2p_config: &P2pConfig,
432440
common_protocol_version: SupportedProtocolVersion,
433441
mut socket_writer: MessageWriter<S, Message>,
434442
mut cmd_receiver: mpsc::UnboundedReceiver<WriterCommand>,
@@ -442,11 +450,12 @@ async fn writer_loop<S: PeerStream>(
442450
message.encoded_size()
443451
);
444452

445-
tokio::time::timeout(SOCKET_WRITE_TIMEOUT, socket_writer.send(*message))
446-
.await
447-
.map_err(|_| {
448-
P2pError::NetworkingError(NetworkingError::SocketWriteTimedOut)
449-
})??;
453+
tokio::time::timeout(
454+
*p2p_config.backend_timeouts.socket_write_timeout,
455+
socket_writer.send(*message),
456+
)
457+
.await
458+
.map_err(|_| P2pError::NetworkingError(NetworkingError::SocketWriteTimedOut))??;
450459
}
451460
WriterCommand::Disconnect { reason } => {
452461
log::debug!("Disconnection requested, the reason is {:?}", reason);

‎p2p/src/peer_manager/tests/connections.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,6 +1004,8 @@ async fn connection_timeout_rpc_notified<T>(
10041004
backend_timeouts: BackendTimeoutsConfig {
10051005
outbound_connection_timeout: Duration::from_secs(1).into(),
10061006
peer_handshake_timeout: Default::default(),
1007+
disconnection_timeout: Default::default(),
1008+
socket_write_timeout: Default::default(),
10071009
},
10081010

10091011
bind_addresses: Default::default(),

‎p2p/src/sync/tests/network_sync.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,8 @@ async fn send_block_from_the_future_again(#[case] seed: Seed) {
426426
backend_timeouts: BackendTimeoutsConfig {
427427
peer_handshake_timeout: Duration::from_secs(1).into(),
428428
outbound_connection_timeout: Default::default(),
429+
disconnection_timeout: Default::default(),
430+
socket_write_timeout: Default::default(),
429431
},
430432

431433
bind_addresses: Default::default(),

‎p2p/src/tests/connection_lockup_when_socket_not_read.rs‎

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@
1313
// See the License for the specific language governing permissions and
1414
// limitations under the License.
1515

16-
use std::sync::Arc;
16+
use std::{sync::Arc, time::Duration};
1717

1818
use itertools::Itertools as _;
1919
use rstest::rstest;
2020

2121
use chainstate::{BlockSource, ChainstateConfig, Locator};
2222
use common::{
2323
chain,
24-
primitives::{Id, Idable as _},
24+
primitives::{user_agent::mintlayer_core_user_agent, Id, Idable as _},
2525
};
2626
use logging::log;
2727
use networking::{
@@ -38,6 +38,7 @@ use test_utils::{
3838
};
3939

4040
use crate::{
41+
config::{BackendTimeoutsConfig, P2pConfig},
4142
message::{HeaderList, HeaderListRequest},
4243
net::{
4344
default_backend::types::{HandshakeMessage, Message, P2pTimestamp},
@@ -258,9 +259,38 @@ async fn timeout_when_socket_not_read(#[case] seed: Seed) {
258259
let channel_max_buf_size = 1000;
259260
let node_headers_count = 100;
260261

262+
let socket_write_timeout = Duration::from_secs(3);
263+
let no_disconnection_after = Duration::from_secs(1);
264+
261265
let time_getter = BasicTestTimeGetter::new();
262266
let chain_config = Arc::new(chain::config::create_unit_test_config());
263-
let p2p_config = Arc::new(test_p2p_config());
267+
let p2p_config = Arc::new(P2pConfig {
268+
backend_timeouts: BackendTimeoutsConfig {
269+
socket_write_timeout: socket_write_timeout.into(),
270+
271+
outbound_connection_timeout: Default::default(),
272+
peer_handshake_timeout: Default::default(),
273+
disconnection_timeout: Default::default(),
274+
},
275+
276+
bind_addresses: Default::default(),
277+
socks5_proxy: Default::default(),
278+
disable_noise: Default::default(),
279+
boot_nodes: Default::default(),
280+
reserved_nodes: Default::default(),
281+
whitelisted_addresses: Default::default(),
282+
ban_config: Default::default(),
283+
ping_check_period: Default::default(),
284+
ping_timeout: Default::default(),
285+
max_clock_diff: Default::default(),
286+
node_type: Default::default(),
287+
allow_discover_private_ips: Default::default(),
288+
user_agent: mintlayer_core_user_agent(),
289+
sync_stalling_timeout: Default::default(),
290+
peer_manager_config: Default::default(),
291+
protocol_config: Default::default(),
292+
custom_disconnection_reason_for_banning: Default::default(),
293+
});
264294

265295
log::debug!("Generating blocks");
266296

@@ -367,6 +397,15 @@ async fn timeout_when_socket_not_read(#[case] seed: Seed) {
367397
}
368398
);
369399

400+
// Wait for a short period of time, there should be no disconnection.
401+
tokio::time::timeout(
402+
no_disconnection_after,
403+
test_node.peer_mgr_notification_receiver().recv(),
404+
)
405+
.await
406+
.unwrap_err();
407+
408+
// But eventually the disconnection should happen when the socket write timeout expires.
370409
log::debug!("Expecting PeerManagerNotification::ConnectionClosed");
371410
let peer_mgr_notif = test_node.peer_mgr_notification_receiver().recv().await.unwrap();
372411
assert_matches!(

0 commit comments

Comments
 (0)