Abort upon panic

[pavel-kokolemin]

Right now tokio runtime catches panics from spawned tasks and just prints something like this (I added the panic! statement for test):

thread 'tokio-runtime-worker' panicked at 'explicit panic', /path/to/p2p/src/net/default_backend/peer.rs:204:9

And the node will continue working after that (same for release and debug builds).
Aborting the process should be a safer choice. If the process is aborted, the core dump should be enough to debug the problem.
This change will affect all binaries in the workspace (node, wallet, dns server).

[stanislav-tkach]

I guess this makes sense unless we want to catch panics somewhere.

[TheQuantumPhysicist]

Please add a comment here explaining why this is being added. I'm sure in a couple of months we'll forget.

[TheQuantumPhysicist]

Please add the comment and feel free to merge.

[iljakuklic]

How does this interact with proptest shrinking? If a proptest fails, it tries to reproduce the failure with smaller inputs to find a small input that still produces the issue. I guess if panicking fails, it will block shrinking from happening. We also lose backtraces if I am not mistaken. Can you check? Could this be done with a tokio settings, or maybe wrapper over tokio::spawn, rather than making it a global change?

[pavel-kokolemin]

How does this interact with proptest shrinking? If a proptest fails, it tries to reproduce the failure with smaller inputs to find a small input that still produces the issue. I guess if panicking fails, it will block shrinking from happening.

Proptest works fine because it's used from the test profile. I just checked it.

We also lose backtraces if I am not mistaken. Can you check?

Works fine (I tested the release build):

thread 'tokio-runtime-worker' panicked at 'explicit panic', mintlayer-core/p2p/src/net/default_backend/peer.rs:204:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/panicking.rs:64:14
   2: core::panicking::panic
             at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/panicking.rs:111:5
   3: p2p::net::default_backend::backend::Backend<T>::create_peer::{{closure}}
   4: tokio::runtime::task::core::Core<T,S>::poll
   5: tokio::runtime::task::harness::Harness<T,S>::poll
   6: tokio::runtime::scheduler::multi_thread::worker::Context::run_task
   7: tokio::runtime::scheduler::multi_thread::worker::Context::run
   8: tokio::runtime::scheduler::multi_thread::worker::run
   9: tokio::loom::std::unsafe_cell::UnsafeCell<T>::with_mut
  10: tokio::runtime::task::core::Core<T,S>::poll
  11: tokio::runtime::task::harness::Harness<T,S>::poll
  12: tokio::runtime::blocking::pool::Inner::run
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Could this be done with a tokio settings, or maybe wrapper over tokio::spawn, rather than making it a global change?

Tokio runtime does not have such setting yet: tokio-rs/tokio#4516
A wrapper for tokio::spawn or aborting the process with std::panic::set_hook should work, but I don't think it's worth the effort.

[iljakuklic]

Thanks for researching the interaction with proptest