Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Panic originating from libp2p_swarm #2373

Closed
pepyakin opened this issue Feb 3, 2021 · 5 comments · Fixed by libp2p/rust-libp2p#1945 or #2384
Closed

Panic originating from libp2p_swarm #2373

pepyakin opened this issue Feb 3, 2021 · 5 comments · Fixed by libp2p/rust-libp2p#1945 or #2384
Assignees

Comments

@pepyakin
Copy link
Contributor

pepyakin commented Feb 3, 2021

While running a simple localnet this happens on alice almost immediately


====================

Version: 0.8.28-071e00165-x86_64-linux-gnu

   0: sp_panic_handler::set::{{closure}}
   1: std::panicking::rust_panic_with_hook
             at /rustc/fde692739576089729885b7f79aa2232cb9778c5/library/std/src/panicking.rs:595
   2: std::panicking::begin_panic::{{closure}}
   3: std::sys_common::backtrace::__rust_end_short_backtrace
   4: std::panicking::begin_panic
   5: <libp2p_swarm::toggle::ToggleProtoHandler<TInner> as libp2p_swarm::protocols_handler::ProtocolsHandler>::inject_listen_upgrade_error
   6: <libp2p_swarm::protocols_handler::select::ProtocolsHandlerSelect<TProto1,TProto2> as libp2p_swarm::protocols_handler::ProtocolsHandler>::inject_listen_upgrade_error
   7: <libp2p_swarm::protocols_handler::select::ProtocolsHandlerSelect<TProto1,TProto2> as libp2p_swarm::protocols_handler::ProtocolsHandler>::inject_listen_upgrade_error
   8: libp2p_core::connection::Connection<TMuxer,THandler>::poll
   9: <libp2p_core::connection::manager::task::Task<F,M,H,I,O,E> as core::future::future::Future>::poll
  10: <sc_service::task_manager::prometheus_future::PrometheusFuture<T> as core::future::future::Future>::poll
  11: <futures_util::future::select::Select<A,B> as core::future::future::Future>::poll
  12: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  13: <tracing_futures::Instrumented<T> as core::future::future::Future>::poll
  14: <sc_service::task_manager::WithTelemetrySpan<T> as core::future::future::Future>::poll
  15: tokio::runtime::task::core::Core<T,S>::poll
  16: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
  17: tokio::runtime::task::harness::Harness<T,S>::poll
  18: std::thread::local::LocalKey<T>::with
  19: tokio::runtime::thread_pool::worker::Context::run_task
  20: tokio::runtime::thread_pool::worker::Context::run
  21: tokio::macros::scoped_tls::ScopedKey<T>::set
  22: tokio::runtime::thread_pool::worker::run
  23: tokio::runtime::task::core::Core<T,S>::poll
  24: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
  25: tokio::runtime::task::harness::Harness<T,S>::poll
  26: tokio::runtime::blocking::pool::Inner::run
  27: tokio::runtime::context::enter
  28: std::sys_common::backtrace::__rust_begin_short_backtrace
  29: core::ops::function::FnOnce::call_once{{vtable.shim}}
  30: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/fde692739576089729885b7f79aa2232cb9778c5/library/alloc/src/boxed.rs:1480
      <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/fde692739576089729885b7f79aa2232cb9778c5/library/alloc/src/boxed.rs:1480
      std::sys::unix::thread::Thread::new::thread_start
             at /rustc/fde692739576089729885b7f79aa2232cb9778c5/library/std/src/sys/unix/thread.rs:71
  31: start_thread
  32: __clone


Thread 'tokio-runtime-worker' panicked at 'Unexpected Either::Right on enabled `inject_listen_upgrade_error`.', /home/lilpep/.cargo/registry/src/jackfan.us.kg-1ecc6299db9ec823/libp2p-swarm-0.27.0/src/toggle.rs:288

This is a bug. Please report it at:

	https://github.com/paritytech/polkadot/issues/new
@tomaka
Copy link
Contributor

tomaka commented Feb 3, 2021

cc @mxinden @romanb

@mxinden
Copy link
Contributor

mxinden commented Feb 3, 2021

Thanks for reporting @pepyakin.

The panic origins here.

From taking a first look there seems to be a mismatch of expectations between ToggleProtoHandler and the outer ProtocolsHandlerSelect in regards to when inject_listen_upgrade_error is called . ToggleProtoHandler is not expecting a call to inject_listen_upgrade_error when disabled (Either::Right), while ProtocolsHandlerSelect forwards most errors (Timer, Timeout, ...) from inject_listen_upgrade_error to both of its sub handlers.

I would expect making ToggleProtoHandler ignore listen upgrade errors when disabled would do the trick, but I want to put a bit more thought into this.

I am surprised that this surfaced only now, given that this code path seems to not have been touched recently.

@mxinden mxinden self-assigned this Feb 4, 2021
@tomaka
Copy link
Contributor

tomaka commented Feb 4, 2021

I am surprised that this surfaced only now, given that this code path seems to not have been touched recently.

It was most likely revealed by paritytech/substrate#7963 (merged yesterday), as it uses Toggle.

@pepyakin
Copy link
Contributor Author

pepyakin commented Feb 4, 2021

Let's keep it open until that fix hits polkadot master.

@pepyakin pepyakin reopened this Feb 4, 2021
@mxinden
Copy link
Contributor

mxinden commented Feb 4, 2021

Let's keep it open until that fix hits polkadot master.

On it.

mxinden added a commit that referenced this issue Feb 4, 2021

Unverified

This user has not yet uploaded their public signing key.
`libp2p-swarm` `CHANGELOG.md`:

- Have `ToggleProtoHandler` ignore listen upgrade errors when disabled.
  [PR 1945](https://github.com/libp2p/rust-libp2p/pull/1945/files).

Fixes:

#2373
mxinden added a commit that referenced this issue Feb 4, 2021

Unverified

This user has not yet uploaded their public signing key.
`libp2p-swarm` `CHANGELOG.md`:

- Have `ToggleProtoHandler` ignore listen upgrade errors when disabled.
  [PR 1945](https://github.com/libp2p/rust-libp2p/pull/1945/files).

Fixes:

#2373
bkchr pushed a commit that referenced this issue Feb 4, 2021

Unverified

This user has not yet uploaded their public signing key.
`libp2p-swarm` `CHANGELOG.md`:

- Have `ToggleProtoHandler` ignore listen upgrade errors when disabled.
  [PR 1945](https://github.com/libp2p/rust-libp2p/pull/1945/files).

Fixes:

#2373
KiChjang pushed a commit to paritytech/xcm that referenced this issue Aug 26, 2023

Unverified

This user has not yet uploaded their public signing key.
`libp2p-swarm` `CHANGELOG.md`:

- Have `ToggleProtoHandler` ignore listen upgrade errors when disabled.
  [PR 1945](https://github.com/libp2p/rust-libp2p/pull/1945/files).

Fixes:

paritytech/polkadot#2373
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants