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

Should call user_handler() in interrupt handler? #31

Closed
rom1v opened this issue Sep 9, 2017 · 2 comments
Closed

Should call user_handler() in interrupt handler? #31

rom1v opened this issue Sep 9, 2017 · 2 comments

Comments

@rom1v
Copy link

rom1v commented Sep 9, 2017

Looking at the source code, the user handler is called from a separate thread, not from the OS interrupt handler.

As a consequence, setting a flag in the user handler and reading it from elsewhere is inherently racy.

For instance, this cannot be written without race conditions:

static INTERRUPTED_BY_USER: AtomicBool = ATOMIC_BOOL_INIT;

fn main() {
    ctrlc::set_handler(move || {
        println!("Ctrl+C detected");
        INTERRUPTED_BY_USER.store(true, Ordering::Release));
    }
    // …
    if let Err(err) = some_io_call() {
        if err.kind() == io::ErrorKind::Interrupted {
            // problem: at this point, INTERRUPTED_BY_USER may not be set by the ctrlc handler yet
            if INTERRUPTED_BY_USER.load(Ordering::Acquire) {
                println!("User requested interruption");
                quit();
            } else {
                println!("Interrupted by some other signal, retry...");
                retry();
            }
        }   
    }
}

Do you see any problems with calling the user handler from the OS interrupt handler?

EDIT: probably related to #30 by the way.

@rom1v rom1v changed the title Should call user_handler() in interrupt handler Should call user_handler() in interrupt handler? Sep 9, 2017
@rom1v
Copy link
Author

rom1v commented Sep 9, 2017

I found this comment:

Running our handler in its own thread is unfortunate, but we cannot safely allow arbitrary rust code to run in the os_handler() since most operations are not async signal safe.

What if ctrlc exposed an unsafe API to register a user handler to be run directly in the os_handler()?

@henninglive
Copy link
Contributor

henninglive commented Sep 9, 2017

That comment is correct, most rust code would be unsafe when called from the OS signal handler on a POSIX system. Signal handlers are special, you can only use special async-signal safe functions, this means no memory allocation, no box or vecs, no panicing and more. Allowing registering of raw OS handlers wouldn’t be very useful, if you absolutely need to do this, you can do the libc call to setup the signal handler yourself. On windows, the OS spins up a new thread for the incoming signals, so here it would actually be safe call arbitrary rust code, but exposing this would not be cross platform.

The safe and cross platfrom solution is to provide an API with an custom safe abstraction over these atomic flag/counter type signal handlers, this can be implement without the extra thread, which means no race. Here is my suggestion #27 for adding this, but I have not had the time implement anything yet.

@Detegr Detegr closed this as completed Oct 13, 2017
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