Skip to content

Add try_set_scoped_handler: Scoped Ctrl-C Handler for Non-'static Closures #132

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

malt03
Copy link

@malt03 malt03 commented May 9, 2025

First of all, thank you for providing such an excellent crate. It has been incredibly helpful in the development of my own CLI tools!


This PR introduces a new function, try_set_scoped_handler, that enables setting a scoped Ctrl-C (SIGINT) signal handler using a std::thread::Scope. Unlike the existing set_handler and try_set_handler APIs, this version allows the use of non-'static closures, making it possible to reference stack-local state without requiring heap allocations or synchronization primitives like Arc.

Motivation

The existing signal handler APIs require 'static closures, which limits their flexibility—especially in multi-threaded code where state is short-lived and scoped. This new function is particularly useful in structured concurrency scenarios (e.g., with thread::scope), where signal handling logic only needs to live within a temporary thread scope.

Highlights

  • Scoped lifetime support: The handler can reference variables that do not outlive the thread scope.
  • Automatic de-registration: The handler can signal that it is finished by returning true, allowing the scope to exit cleanly.
  • Error on multiple registrations: Consistent with the existing API, only one handler (scoped or static) may be installed at any time.

Example

use std::sync::atomic::{AtomicBool, Ordering};
use std::thread;

let flag = AtomicBool::new(false);
thread::scope(|s| {
    ctrlc::try_set_scoped_handler(s, || {
        flag.store(true, Ordering::SeqCst);
        true
    }).unwrap();

    // Perform scoped work...
});

Safety and Semantics

  • The handler must eventually return true to allow the scope to complete.
  • This function ensures that signal handling remains single-registration and safe within a scoped thread context.

Limitations

  • Only one scoped or static handler may be registered at a time.
  • The scoped handler cannot be replaced, even if it finishes execution.

@Detegr
Copy link
Owner

Detegr commented May 11, 2025

Hi.

Thanks for the PR, it is well documented and the code looks good by first glance. However, like with other PRs that want to add extra functionality I'm questioning whether the benefits outweigh the added complexity. Do you have some concrete use cases of signal handling where this kind of scoped handling would be needed?

@malt03
Copy link
Author

malt03 commented May 12, 2025

Hi, thank you for your thoughtful feedback.

You’re absolutely right to ask whether the added functionality justifies the increased complexity—this is a fair and important question when maintaining a crate like ctrlc.

To be honest, I don’t have a strong performance-based argument for this addition. The difference in real-world applications is likely negligible in most cases.

However, from the perspective of idiomatic Rust usage and ergonomics, I believe this functionality fills a meaningful gap. Specifically, when writing concurrent code with thread::scope, I find it frustrating that I need to reach for Arc or other synchronization mechanisms even when they are not conceptually necessary, just to satisfy 'static bounds imposed by the current API.

This leads to code that:

  • Requires additional mental overhead
  • Feels less “Rusty” (i.e., it breaks the expectation that the borrow checker and lifetimes should let me express local, scoped lifetimes cleanly without heap allocations when not needed)

I understand this might sound like a “purist” or stylistic argument, but for those of us who care deeply about expressing correctness and lifetime boundaries precisely, being forced into 'static when we could do better is genuinely disappointing.

I hope this helps clarify the motivation. I’d be happy to further refine the design or documentation if needed.

Thanks again for your consideration.

@Detegr
Copy link
Owner

Detegr commented May 14, 2025

Specifically, when writing concurrent code with thread::scope, I find it frustrating that I need to reach for Arc or other synchronization mechanisms even when they are not conceptually necessary, just to satisfy 'static bounds imposed by the current API.

I get the idea, but let's stop here for a minute. The very idea of handling signals differently for different threads is very advanced and I doubt it's very rarely used in practice. If you have multiple threads, then you'd want to set the signal masks appropriately because it's not defined to which thread of an application the OS sends the signal to, unless you specify it yourself or use signal masks to define the behavior.

Such advanced use is out of the scope of this crate. This crate is just for handling the simple case of SIGINT and SIGTERM and I want to keep it like that. As the readme states, there are better crates available for more advanced use cases.

@malt03
Copy link
Author

malt03 commented May 14, 2025

Thanks for the clarification — I see where you're coming from.

Just to be clear, I’m not trying to introduce advanced per-thread signal handling or mess with signal masks. I’m simply hoping to make it easier to write scoped, idiomatic Rust without reaching for Arc just to satisfy a 'static bound. That’s really all there is to it.

To illustrate this more concretely, here’s a comparison of what I’m talking about:

Current (with 'static bound and Arc workaround)

fn main() {
    let flag = Arc::new(AtomicBool::new(true)); // <- not the Rust I signed up for...
    let flag_clone = flag.clone(); // <- we clone, not because we want to, but because we must

    ctrlc::set_handler(move || {
        flag_clone.store(false, Ordering::SeqCst);
    })
    .unwrap();

    thread::scope(|s| {
        for _ in 0..8 {
            s.spawn(|| {
                while flag.load(Ordering::SeqCst) {
                    do_something();
                }
            });
        }
    });
}

Proposed (scoped)

fn main() {
    let flag = AtomicBool::new(true); // <- this is the Rust I signed up for ❤️

    thread::scope(|s| {
        ctrlc::try_set_scoped_handler(s, || {
            flag.store(false, Ordering::SeqCst);
            true
        })
        .unwrap();

        for _ in 0..8 {
            s.spawn(|| {
                while flag.load(Ordering::SeqCst) {
                    do_something();
                }
            });
        }
    });
}

That said, I completely respect your desire to keep ctrlc focused on the simplest possible use case. If this change feels out of scope for the crate’s philosophy, I totally understand. I really appreciate the thoughtful feedback and discussion either way.

@Detegr
Copy link
Owner

Detegr commented May 14, 2025

Thanks for the example, this is what I was looking for. I misunderstood the benefits this would provide, sorry about that.

The example indeed seems nice in comparison. I will take a look at this a bit deeper level once I have the time. Thank you for the patience.

@malt03
Copy link
Author

malt03 commented May 14, 2025

Glad to hear that — and thanks again for maintaining such a solid crate!

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

Successfully merging this pull request may close these issues.

2 participants