hyperledger / iroha

Iroha - A simple, enterprise-grade decentralized ledger
https://wiki.hyperledger.org/display/iroha
Apache License 2.0
434 stars 277 forks source link

Improve handling of task panics #4698

Closed 0x009922 closed 3 weeks ago

0x009922 commented 4 months ago

Currently Iroha spawns many tokio::tasks in a detached way, without checking whether they panic somewhere along execution.

Here are a few examples (just cases when the handle is completely ignored; there are many other when the handle is used, but not to check whether it resolves in an error):

https://github.com/hyperledger/iroha/blob/5085ff2435bf2e412dec76649b19d3a7bf091239/p2p/src/network.rs#L111

https://github.com/hyperledger/iroha/blob/f5e3c493a6f2f336da09d75c8d00562aac8168a5/cli/src/lib.rs#L387

https://github.com/hyperledger/iroha/blob/f5e3c493a6f2f336da09d75c8d00562aac8168a5/cli/src/lib.rs#L137

https://github.com/hyperledger/iroha/blob/f5e3c493a6f2f336da09d75c8d00562aac8168a5/cli/src/lib.rs#L494

It has a few issues:

For example, in case of the NetworkBase task panicking, we don't see anything except NetworkBase must accept messages until there is at least one handle to it: SendError { .. } (which doesn't tell the cause).

Instead, it would be good:

Tools to help: TaskTracker, JoinSet, CancellationToken.

Erigara commented 4 months ago

There is also a issue that panics SHOULD be detected by panic set_hook which should print panic and shutdown iroha, but there is problems with it.