Skip to content

Commit

Permalink
ping: Make ping fault tolerant wrt outbound substreams races (#133)
Browse files Browse the repository at this point in the history
We have discovered a race between the TransportService and Protocol
interface.

This issue was first seen on the identify protocol:
- #103

After looking a bit closely into it, indeed there might be a chance for
the following to happen:
- T0: Transport service sends `TransportEvent::ConnectionEstablished`
and `TransportEvent::SubstreamOpened`
- T1: The peer disconnects
- T2: Ping protocol receives an `TransportEvent::ConnectionEstablished`
event for a disconnected peer
- T3: `Ping::on_connection_established` fails to open a substream and
`pending_open` is never populated
- T4: Ping protocol receives an `TransportEvent::SubstreamOpened` event
which would have panicked litep2p

This PR replaces the `todo!` in case of the race with a warning, and
we'll no longer panic.

It is still good to capture those as warnings for visibility, even if
there's no implied side-effect on our code.

Signed-off-by: Alexandru Vasile <[email protected]>
  • Loading branch information
lexnv authored May 30, 2024
1 parent 467a457 commit ec31635
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/protocol/libp2p/ping/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,11 @@ impl Ping {
self.on_outbound_substream(peer, substream_id, substream);
}
None => {
todo!("substream {substream_id:?} does not exist");
tracing::warn!(
target: LOG_TARGET,
?substream_id,
"outbound ping substream ID does not exist",
);
}
}
}
Expand Down

0 comments on commit ec31635

Please sign in to comment.