Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MultipleHandler's using nohup to run binary #103

Closed
okynos opened this issue May 27, 2023 · 8 comments
Closed

MultipleHandler's using nohup to run binary #103

okynos opened this issue May 27, 2023 · 8 comments

Comments

@okynos
Copy link

okynos commented May 27, 2023

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
Copy link
Owner

Detegr commented May 28, 2023

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
Copy link
Author

okynos commented May 28, 2023

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
Copy link
Owner

Detegr commented May 29, 2023

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
Copy link

ngc0202 commented May 30, 2023

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
Copy link
Owner

Detegr commented May 30, 2023

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

@Detegr
Copy link
Owner

Detegr commented May 30, 2023

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

@Detegr Detegr closed this as completed May 30, 2023
@Detegr
Copy link
Owner

Detegr commented May 30, 2023

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
Copy link
Author

okynos commented May 31, 2023

Thanks @Detegr. Will review my requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants