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

Fix race condition in stopping FSEvent runner #40

Merged
merged 1 commit into from
Dec 28, 2012
Merged

Fix race condition in stopping FSEvent runner #40

merged 1 commit into from
Dec 28, 2012

Conversation

dbussink
Copy link
Contributor

The race condition is the following. When a FSEvent#stop is called, it
kills the child process and closes the pipe. It can happen that the kill
signal is sent and then succesfully the pipe is closed, before the child
process dies and the pipe in the parent is closed.

This means that IO::select() can raise a Errno::EBADF if the file
descriptor is closed while waiting in select(). This additional rescue
makes sure we shutdown in this case just like any others. This is a much
more reliable solution than adding for example a sleep() between
Process.kill and @pipe.close in FSEvent#stop.

Found because of rubinius/rubinius#2102.

The race conditionn is the following. When a FSEvent#stop is called, it
kills the child process and closes the pipe. It can happen that the kill
signal is sent and then succesfully the pipe is closed, before the child
process dies and the pipe in the parent is closed.

This means that IO::select() can raise a Errno::EBADF if the file
descriptor is closed while waiting in select(). This additional rescue
makes sure we shutdown in this case just like any others. This is a much
more reliable solution than adding for example a sleep() between
Process.kill and @pipe.close in FSEvent#stop.

Found because of rubinius/rubinius#2102.
@ttilley
Copy link
Member

ttilley commented Dec 28, 2012

Good catch. Thanks. :)

ttilley added a commit that referenced this pull request Dec 28, 2012
Fix race condition in stopping FSEvent runner
@ttilley ttilley merged commit fa8ad08 into guard:master Dec 28, 2012
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.

None yet

2 participants