Detegr/rust-ctrlc

Ctrl-C while waiting for IO

Closed this issue · 10 comments

ctrlc doesn't work as I would expect it to when SIGINT is sent while the main thread blocks waiting for IO. Consider this example:

use std::net::TcpListener;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};

extern crate ctrlc;

fn main() {
    let sigint = Arc::new(AtomicBool::new(false));
    let sigint_clone = sigint.clone();
    ctrlc::set_handler(move || {
            sigint_clone.store(true, Ordering::SeqCst);
        })
        .expect("Error setting SIGINT handler!");

    let listener = TcpListener::bind("127.0.0.1:8080").unwrap();

    while !sigint.load(Ordering::SeqCst) {
        // blocking IO call
        let (_, _) = listener.accept().unwrap();
    }
    println!("Gracefully exiting...");
}

If I run this code and press Ctrl-C, I get my command line back immediately (without "Gracefully exiting..." being printed) and the main thread hangs somewhere in the background, waiting to accept() a TCP connection. (NB that you can't re-run the program at this point because the address is still in use.) If I then establish a connection (using e.g. curl localhost:8080), it is accepted by the listener, the thread resumes, the message is printed and the program exits.

The same thing happens if you wait for user input using io::stdin().read_line(), only this case is of course resolved by typing anything + Enter at the same command line after the Ctrl-C. This leads me to think this is a general property of how ctrlc interacts with blocking IO.

I would argue it would be more intuitive / useful / generally preferable if ctrlc were able to terminate any pending IO, so that the shutdown can be completed. On the other hand, I understand there might be no way to achieve this in Rust. So if there's no workaround for this, I suggest it would be nice to mention this limitation in the docs.

Also, thanks for an otherwise very useful crate! :)

Or maybe there's a way for users of the crate to handle this in their own code using select or some similar mechanism...? In which case it would be awesome if the docs had an example of how to do that for us newbies :)

This crate tries to do cross-platform ctrl-c handling, using signal deliver and EINTR as a way to interrupt blocking system calls is not cross-platform at all. The cross-platform way of doing this is with futures. Here is a crate which implements signals as a futures::stream::Stream. You can use this to select between I/O streams and a signal stream, allowing you to abort when a signal is received.

If that doesn’t work, you can use a self-pipe and select() as you said, but you need to do everything yourself.

extern crate nix;

use nix::sys::signal;
use nix::sys::select;
use nix::{Error, Errno};

use std::io::Write;
use std::net::TcpListener;
use std::os::unix::io::{RawFd, AsRawFd};

static mut PIPE: (RawFd, RawFd) = (-1, -1);

extern fn signal_handler(_: nix::libc::c_int) {
    // Signal handlers are special functions, only [async-signal-safe]
    // (http://man7.org/linux/man-pages/man7/signal-safety.7.html) functions
    // can be called in this contex.
    unsafe {
        nix::unistd::write(PIPE.1, &[0u8]).is_ok();
    }
}

fn main() {
    // Create a new non-blocking pipe that we write to from our signal handler.
    let ctrlc_fd = unsafe {
        PIPE = nix::unistd::pipe2(nix::fcntl::O_CLOEXEC | nix::fcntl::O_NONBLOCK).unwrap();
        PIPE.0   
    };
    
    // Set-up our signal handler.
    let handler = signal::SigHandler::Handler(signal_handler);
    let action = signal::SigAction::new(handler,
        signal::SA_RESTART,
        signal::SigSet::empty()
    );
    unsafe {
        signal::sigaction(signal::Signal::SIGINT, &action).unwrap();
    }
    
    let listener = TcpListener::bind("127.0.0.1:8080").unwrap();
    
    // We can retive underlining file descriptors with std::os::unix::io::AsRawFd
    let listener_fd = listener.as_raw_fd();
    
    // File descriptors are intergers, select needs to know highest numerical 
    // value plus one of all file descriptors.
    let nfds = if ctrlc_fd > listener_fd { ctrlc_fd + 1} else {listener_fd + 1};
    
    let mut fd_set = select::FdSet::new();

    loop {
        // Wait for either socket or pipe to be ready for read, no timeout. 
        fd_set.insert(ctrlc_fd);
        fd_set.insert(listener_fd);
        
        match select::select(nfds, Some(&mut fd_set), None, None, None) {
            Ok(0) => unreachable!("timeout"),
            Ok(_) => {
                // Socket is ready.
                if fd_set.contains(listener_fd) {
                    let (mut con, addr) = listener.accept().unwrap();
                    println!("Connection {}", addr);
                    con.write_all(&b"HTTP/1.1 404 Not Found\n\n"[..]).unwrap();
                }

                // Pipe is ready.
                if fd_set.contains(ctrlc_fd) {
                    println!("Gracefully exiting...");
                    break;
                }
            },
            Err(Error::Sys(Errno::EINTR)) => {},
            Err(e) => panic!("{}", e),
        }
    }
}

@henninglive Thank you very much! The select solution is very much beyond anything I'd be able to come up with (as a newcomer to Rust and someone only vaguely aware of systems programming), but it's very instructive nonetheless :) Thanks also for the pointer to the tokio-signal crate, I'll have a look at the futures API and try to come up with a solution using these.

So in general, there's no way to have ctrlc itself take care of this -- it's the responsibility of the calling code, right? In that case, I think a short discussion and a code example would be a welcome addition to the documentation (as per the OP).

(Not implying you have to be the one to write those, just explaining why I'm not closing the issue.)

I agree that this should be documented. I may write it if I have time. Other people have raised the same problem before. We have discussed(#16, #20) a solution where we would allow the user to disable SA_RESTART. This would make most system calls interruptible by signal delivery, but this will only interrupt the system call on whatever thread ended up handling the signal. In order to use this to interrupt I/O you need to make sure the signal is deliver to the thread you want to interrupt. Signal delivery order to threads is undefined under POSIX. On Linux the main thread is preferred and undefined for the rest. To make this work you must funnel signals by masking/blocking signals with pthread_sigmask() on the other threads that you don’t want to interrupt.

This is a terrible way to interrupt I/O, but more importantly this requires special care by user for them to use it correctly, at which point they should probably be doing the libc calls themselves to ensure everything is setup correctly. We therefore choose not implement it.

The better way is to use a self-pipe with select() or poll(), but preferably you would have a safe cross-platform abstraction on top. tokio and tokio-signal is essentially this.

Here is the equivalent code using tokio and tokio-signal for future documentation.

extern crate futures;
extern crate tokio_core;
extern crate tokio_signal;
extern crate tokio_io;

use futures::{Stream, Future};
use tokio_core::reactor::Core;
use tokio_core::net::TcpListener;
use std::net::SocketAddr;
use std::io::{Error, ErrorKind};

enum Ready {
    Connection,
    CtrlC,
}

#[derive(Debug)]
struct CtrlC;

impl std::fmt::Display for CtrlC {
    fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
        use std::error::Error;
        f.write_str(self.description())
    }
}

impl std::error::Error for CtrlC {
    fn description(&self) -> &str {
        "Keyboard Interrupt"
    }
}

fn main() {
    // Set up a Tokio event loop
    let mut core = Core::new().unwrap();

    // Listen for tcp connection, returns a stream of connections.
    let addr = "127.0.0.1:8080".parse::<SocketAddr>().unwrap();
    let listener = TcpListener::bind(&addr, &core.handle())
        .unwrap()
        .incoming();

    // Handle each connection in the stream
    let http = listener.and_then(|(tcp, addr)| {
        println!("Connection {}", addr);
        tokio_io::io::write_all(tcp, &b"HTTP/1.1 404 Not Found\n\n"[..])
            .map(|(_, _)| Ready::Connection)
        // write_all() returns the connection and the buffer used,
        // We throw these away.
    });

    // Stream of Ctrl+C events
    let ctrlc = tokio_signal::ctrl_c(&core.handle())
        .flatten_stream()
        .map(|_| Ready::CtrlC);

    // Combine the streams by selecting from either stream.
    // the streams must return the same types for this work.
    // for_each() runs the stream to completion or until a error occurs,
    // it returns a future.
    let select = http.select(ctrlc)
        .for_each(|ready| {
            // Handle Ctrl+C as an error to ensure early exit.
            // The future must return std::io::Error, so we must wrap
            // our ctrl+c error with an std::io::Error.
            match ready {
                Ready::Connection => Ok(()),
                Ready::CtrlC => Err(Error::new(ErrorKind::Other, CtrlC)),
            }
        });

    // Up until now, we haven't really done anything, just prepared
    // now it's time to actually schedule, and thus execute,
    // on our event loop
    match core.run(select) {
        Ok(_) => unreachable!("Should have run forever"),
        Err(e) => {
            // Exit gracefully on Ctrl+C Error, panic for other errors.
            match (e.get_ref(), e.kind()) {
                (Some(i), ErrorKind::Other) if i.is::<CtrlC>() =>
                    println!("Gracefully exiting..."),
                _ => panic!("{}", e),
            }
        }
    }
}

@henninglive Wow, this has been really instructive, especially thanks to the detailed comments! I can't thank you enough :)

Eventually, it would be nice to put this into the documentation for other users' benefit, with a note to the effect of "if you want to multiplex signal handling with (different types of) blocking input, this type of infrastructure is necessary".

As far as I'm concerned, this can be closed, but maybe you'd like to keep it around while deciding whether (and if so, then where) to put it in the docs?

Would using #27 and #28 fix this issue as well? Then it would be the user's responsibility to handle a thread that checks the value and could terminate the whole program with std::process::exit or the likes.

I've create a wrapper Read and Write type that simply checks that flag is set on each read/write call, and additionally when nested call return ErrorKind::Interrupted-error. And when it is set, returns ErrorKind::Interrupted-kind io::Error, but wrapped into ErrorKind::Other-kind io::Error to avoid ErrorKind::Interrupted-retries in the standard library and other libraries, and enforce its propagation.

Meet the interruptable crate: https://crates.io/crates/interruptable .

Detegr commented

Closed as stale.