ctrlc::Channel

[henninglive]

One problem that has been mentioned(#24, #22) is that there is currently no way to unregister a signal handler. I suggest we solve this by building a new abstraction. This new abstraction would be a struct called something like ctrlc::Channel. Creating a new one would register a native signal handler and create a pipe/channel that we write to in the native signal handler. It would provide recv() and recv_timeout() methods for blocking on and reading from the pipe/channel. Last but not least, it would provide methods for unregistering the handler and destroying itself, this would be called on drop. It would implement Send, but not Sync.

I feel like this interface and the interface I suggested in #27 fits better with Rust’s philosophy of safe abstractions and lifetimes. It would also transfer the responsibility of handling threads, closures and panics to the user. Users can avoid the extra thread if they want and it would fix the current problem with panics in the user provided closure. We would still keep the old API, but we would reimplement it on top of this abstraction.

I suggest we also implement #26, so the ctrlc::Channel would return signal type when read from.

[Detegr]

I do like both of these proposals (Channel and Counter). :+1:

[henninglive]

I will try to submit implementations later this week.

[henninglive]

What are your thoughts on #26, should we add a “Signal” enum. It would make sense for the channel to return something on read. If so what should is look like?

#[repr(u8)]
enum Signal {
    SIGINT, // Should CTRL+C and CTRL+BREAK map to SIGINT?
    SIGTERM,
    //Should we add more?
}

[Detegr]

Commented on #26

[dusty-phillips]

Just a note for when you get back to this. I've been experimenting with the counter+channel_wip branch and discovered that some version #11 is still an issue when you use multiple threads under Windows 10.

Here's a test case:

use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;
use std::thread::{sleep, spawn};
use std::time::Duration;

fn main() {
    let channel = ctrlc::Channel::new(ctrlc::SignalType::Ctrlc).unwrap();

    let interrupted = Arc::new(AtomicBool::new(false));
    let ctrlc_interrupted = interrupted.clone();

    let my_thread = spawn(move || {
        while !interrupted.load(Ordering::SeqCst) {
            sleep(Duration::from_secs(1));
            println!("   in loop...");
        }
        println!("   broke out of the loop in thread");
    });

    println!("waiting for Ctrl-C...");
    channel.recv().unwrap();
    println!("got the signal, exiting...");

    ctrlc_interrupted.store(true, Ordering::SeqCst);
    println!("stored atomic, sleeping");
    sleep(Duration::from_secs(3));
    println!("awakening");
    my_thread.join().unwrap();
    println!("done");
}

Output is as expected when running .\target\debug\bin.exe:

waiting for Ctrl-C...
   in loop...
   in loop...
   in loop...
got the signal, exiting...
stored atomic, sleeping
   in loop...
   broke out of the loop in thread
awakening
done

However, when using cargo run, it terminates too early:

waiting for Ctrl-C...
   in loop...
   in loop...
   in loop...
got the signal, exiting...
stored atomic, sleeping

Not sure if this is related, but under Windows Subsystem for Linux, both cargo run and ./target/debug/bin crash with a panic.

[Detegr]

Thanks for letting me know! The Channel implementation for Windows is flawed at the moment, as WaitForSingleObject does not support pipes and I'm trying to use them anyway (should've read the documentation more carefully). I just haven't had the time to rewrite the implementation.

That being said, I'm not sure if this is related to that in any way, so I'll need to check this out after the reimplementation.

[Detegr]

I did not notice the multiple thread issue anymore in the reimplementation of Channel that now exists in counter+channel branch.

[Detegr]

No one seems to be interested enough in reviewing my code in the pull request, which is completely understandable. It's been multiple years now so I'm thinking the counter+channel code will never be merged, so I'll close this and #27.