Detegr / rust-ctrlc

Easy Ctrl-C handler for Rust projects
https://crates.io/crates/ctrlc
Other
599 stars 79 forks source link

`Init("sem_init failed")` on macOS #14

Closed MasonRemaley closed 7 years ago

MasonRemaley commented 7 years ago

I tried to run the example program from the docs on macOS 10.12.1, but the call to set_handler fails with Init("sem_init failed").

I started to try to look into why this is but got a little confused--it looks like this error is raised when sem_init returns -1, so I tried to look up the docs for sem_init to see what can lead to -1 being returned. It's possible I'm looking at the wrong docs but it seems like it can return three different potential error values (EINVAL, ENOSPC, ENOSYS, or EPERM) none of which are defined as -1, which clearly isn't true since it's returning -1 for me.

henninglive commented 7 years ago

sem_init() will always return -1 on failure, you have to read errnoto get the actual error code.

henninglive commented 7 years ago

I am guessing sem_init() fails with ENOSYS. Unnamed semaphores are apparently not supported on OSX, we could use named semaphores(sem_open() instead of sem_init())

henninglive commented 7 years ago

After looking in to this I believe named semaphores is probably not what we want. Named semaphores are shared between all process and have kernel persistence which means a semaphore will exist until the system is shut down. We should look into the semaphores in Apple's Grand Central Dispatch library on OS X. Here is rust wrapper for libdispatch with bindings for semaphores.

henninglive commented 7 years ago

I haven’t found any documentation that specify if dispatch_semaphore_signal() is async-signal safe or not, but I have found other software(sem_post() is a macro) that call dispatch_semaphore_signal() from signal handlers.

henninglive commented 7 years ago

Here is the source for Grand Central Dispatch and dispatch_semaphore_signal(). It uses "mach/semaphore.h" internally, but based on the documentation I am sceptical to using them in signal handlers.

Semaphores can be used any place where mutexes can occur. This precludes their use in interrupt handlers or within the context of the scheduler, and makes it strongly discouraged in the VM system

Detegr commented 7 years ago

This makes me think that the suggestion suggested in issue #12 may be the most compatible way to handle this. We could stick with the current semaphore implementation on Windows and go with the pipe trick on unixes. @MasonRemaley as a workaround you can use the spinloop version (version 2.x).

Detegr commented 7 years ago

I threw together the implementation suggested in #12 to a branch called pipe. I would appreciate if you could try it out on macOS.

MasonRemaley commented 7 years ago

Thanks! It doesn't build for me because macOS doesn't seem to have pipe2. I'm not familiar with these APIs, this is mostly translated from a random StackOverflow answer so I don't know if it's actually correct or not, but I replaced the call to pipe2 with the following and seems to work so I think something along these lines is probably the solution:

if pipe(pipe_fds) == -1 {
    return Err(Error::Init(format!("pipe failed with error {}", *errno_location())));
}
if fcntl(*pipe_fds.offset(0), F_SETFD, FD_CLOEXEC) == -1 {
    return Err(Error::Init(format!("fcntl failed with error {}", *errno_location())));
}
if fcntl(*pipe_fds.offset(1), F_SETFD, FD_CLOEXEC) == -1 {
    return Err(Error::Init(format!("fcntl failed with error {}", *errno_location())));
}

[EDIT] I also verified that 3.0.0 master is returning ENOSYS as expected, and that 2.0.0 does work for me for now.

Detegr commented 7 years ago

That's right, stupid me. Setting the flags with fcntl should to the trick, just like you have done. I'll do some more testing tomorrow and update the crate if no other issues are found.

Detegr commented 7 years ago

Fixed in 3.0.1.