Detegr / rust-ctrlc

Easy Ctrl-C handler for Rust projects
https://crates.io/crates/ctrlc
Other
596 stars 78 forks source link

MultipleHandler's using nohup to run binary #103

Closed okynos closed 1 year ago

okynos commented 1 year ago

Hello!

I have detected a weird result using your crate. I am programming a File monitor tool in Rust just for context.

Suddenly, my CI tests failed to give this stack trace:

thread 'main' panicked at 'Error setting Ctrl-C handler: MultipleHandlers': src/monitor.rs:165
   0: <backtrace::capture::Backtrace as core::default::Default>::default
   1: log_panics::Config::install_panic_hook::{{closure}}
   2: std::panicking::rust_panic_with_hook
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:702:17
   3: std::panicking::begin_panic_handler::{{closure}}
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:588:13
   4: std::sys_common::backtrace::__rust_end_short_backtrace
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/sys_common/backtrace.rs:138:18
   5: rust_begin_unwind
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:584:5
   6: core::panicking::panic_fmt
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/panicking.rs:142:14
   7: core::result::unwrap_failed
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/result.rs:1785:5
   8: fim::monitor::monitor::{{closure}}
   9: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  10: tokio::runtime::park::CachedParkThread::block_on
  11: tokio::runtime::scheduler::multi_thread::MultiThread::block_on
  12: tokio::runtime::runtime::Runtime::block_on
  13: fim::main
  14: std::sys_common::backtrace::__rust_begin_short_backtrace
  15: std::rt::lang_start::{{closure}}
  16: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/ops/function.rs:283:13
      std::panicking::try::do_call
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:492:40
      std::panicking::try
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:456:19
      std::panic::catch_unwind
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panic.rs:137:14
      std::rt::lang_start_internal::{{closure}}
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/rt.rs:148:48
      std::panicking::try::do_call
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:492:40
      std::panicking::try
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:456:19
      std::panic::catch_unwind
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panic.rs:137:14
      std::rt::lang_start_internal
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/rt.rs:148:20
  17: main
  18: __libc_start_main
  19: <unknown>

After a lot of research, I detected that the ctrlc crate is detecting multiple handlers to something like background execution. The FIM process is called with & in some of the failed tests but from my shell, the error doesn't appear at all. The only way to reproduce this issue is by using the nohup command to start up FIM. Then the error is always present. In this link, you have the failing code and the solution applied now: Failing code: https://github.com/Achiefs/fim/blob/90b39d5e53b3c2952fc4ad09e14f57ea3ad8764a/src/monitor.rs#L152-L164 Partial solution: https://github.com/Achiefs/fim/blob/9455ce7a6d6fee4f91a5a6a6021cfca58c7595b2/src/monitor.rs#L152-L167

Apart from that, I want to know if there is some solution, workaround or a way to manage the SIGINT signal when nohup is used. I think this is related to nohup attaching the FIM process to the init process (That probably handles SIGINT). But I am not an expert in this field.

It also fails on Github actions using & more info here: https://docs.github.com/en/actions/managing-workflow-runs/canceling-a-workflow#steps-github-takes-to-cancel-a-workflow-run

Detegr commented 1 year ago

Hello. Could you provide a minimal example that triggers this? Running the tests with nohup works fine on my x86_64-linux machine.

With tokio I think it would be a good idea to use tokio::signal instead of this crate.

okynos commented 1 year ago

Thanks for your reply. Did you test with FIM 0.4.6? I will perform some tests and come back with the steps to reproduce it. Apart from that thanks for let me know about tokio-signal I didn't know it.

Detegr commented 1 year ago

I did not run FIM, as it has so much code that it's not trivial to pinpoint the issue. If you can come up with a simpler example I could debug why the issue is happening.

ngc0202 commented 1 year ago

After updating my dependencies, I too am encountering this issue when using nohup. This didn't used to be an issue and we've always used nohup with this code. My case seems very simple, the set_handler call is the very first thing that happens. Nonetheless, my attempts at a standalone test case have all run fine so far.

Detegr commented 1 year ago

The termination feature does trigger this, as it's handling SIGHUP, and so is nohup.

Detegr commented 1 year ago

I'm reverting the implementation for #98 and moving it to ctrlc::try_set_handler instead.

Detegr commented 1 year ago

Please update to 3.4.0 to get the overwriting functionality back. I'd also suggest checking whether you really need the termination feature or not.

okynos commented 1 year ago

Thanks @Detegr. Will review my requirements.