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 signal handling for Ruby 2 -- shutdown child processes #209

Merged
merged 7 commits into from
May 5, 2015

Conversation

evanbattaglia
Copy link

Ruby 2 complains when using Mutex#synchronize within in a trap context.
This is because, if the program flow is interrupted by a signal while
the program is in a synchronize block, and synchronize is called from
the trap context, syncrhonize will never return because it will wait for
the same process/thread to release the block.

Here, in a trap context, if we fail to get the lock, I add the signal to a signal queue
which will be run soon after the synchronize block in the normal program
flow is finished. If we can get the lock, we should shutdown / process
the signal immediately, because main program flow is probably just
waiting for the child process and so can not timely check the signal
queue.

Addresses #202

I have checked to make sure the spec I wrote passes with my new code and does not pass with Ruby 2.1.5 using the old code.
Some other qless specs are failing for me in all branches, including master, but it looks like Travis is OK.

@myronmarston @danlecocq @benkirzhner
@tpickett66

Evan Battaglia added 4 commits February 11, 2015 09:42
Ruby 2 complains when using Mutex#synchronize within in a trap context.
This is because, if the program flow is interrupted by a signal while
the program is in a synchronize block, and synchronize is called from
the trap context, syncrhonize will never return because it will wait for
the same process/thread to release the block.

Here, in a trap context, if we fail to get the lock, I add the signal to a signal queue
which will be run soon after the synchronize block in the normal program
flow is finished. If we can get the lock, we should shutdown / process
the signal immediately, because main program flow is probably just
waiting for the child process and so can not timely check the signal
queue.

Addresses #202
ruby -ryaml -rqless raises stack overflow error
@evanbattaglia
Copy link
Author

there seems to be an intermittent spec failure in travis
https://travis-ci.org/seomoz/qless/jobs/50413427
https://travis-ci.org/seomoz/qless/jobs/42763100

@evanbattaglia
Copy link
Author

Myron suggested using a thread-safe queue, and Vadim pointed out that signals can arrive in the middle of an operation on @signal_queue¸ potentially leaving @signal_queue in a weird state. I will change @signal_queue to a rhread-safe queue (if Ruby lets me use it in a trap context)

@evanbattaglia
Copy link
Author

@vadim-moz I updated this to use a Thread-safe queue. Test manually and behavior still looks OK, and my spec still passes

@evanbattaglia
Copy link
Author

Cool looks good. I agree it's nicer just adding the block to the queue!

vadim-ex pushed a commit that referenced this pull request May 5, 2015
Fix signal handling for Ruby 2 -- shutdown child processes
@vadim-ex vadim-ex merged commit 4434669 into master May 5, 2015
@vadim-ex vadim-ex deleted the ruby2-signals-cleanshutdown-new branch May 5, 2015 19:12
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.

2 participants