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 racy shutdown #25

Merged
merged 1 commit into from
Dec 24, 2014
Merged

Fix racy shutdown #25

merged 1 commit into from
Dec 24, 2014

Conversation

petergoldstein
Copy link
Contributor

This is basically a port of Sidekiq PR #2056 - sidekiq/sidekiq#2056 . It includes:

  1. Upgrading the Celluloid version
  2. Adding a Condition variable to address the racy shutdown problem
  3. Updating a spec to account for the change in the Shoryuken::Manager initialize method signature.

…of Sidekiq PR #2056 that includes Celluloid upgrade.
@phstc
Copy link
Collaborator

phstc commented Dec 17, 2014

Awesome. I'm out (= limited Internet connection) until the weekend. As soon
as I get back I will review it. Thanks a lot!

On Dec 14, 2014 6:19 PM, "Peter Goldstein" notifications@github.com wrote:

This is basically a port of Sidekiq PR #2056 - sidekiq/sidekiq#2056 . It
includes:

Upgrading the Celluloid version
Adding a Condition variable to address the racy shutdown problem
Updating a spec to account for the change in the Shoryuken::Manager
initialize method signature.


You can merge this Pull Request by running

git pull https://github.com/petergoldstein/shoryuken
feature/fix_racy_shutdown

Or view, comment on, or merge it at:

#25

Commit Summary
Fix racy shutdown that was copied from Sidekiq's racy shutdown. Port of
Sidekiq PR #2056 that includes Celluloid upgrade.
File Changes
M lib/shoryuken/launcher.rb (6)
M lib/shoryuken/manager.rb (7)
M shoryuken.gemspec (2)
M spec/shoryuken/manager_spec.rb (6)
Patch Links:
https://github.com/phstc/shoryuken/pull/25.patch
https://github.com/phstc/shoryuken/pull/25.diff


Reply to this email directly or view it on GitHub.

@leemhenson
Copy link
Contributor

You'll want to cherry-pick musicglue@4c0e500 too.

@phstc phstc merged commit 734c76d into ruby-shoryuken:master Dec 24, 2014
@phstc
Copy link
Collaborator

phstc commented Dec 24, 2014

Thanks @leemhenson and @petergoldstein, PR added to master 🍻

I was having a hard time trying to understand why the launcher_spec (to run: SPEC_ALL=true rspec) was failing - cherry picked musicglue@4c0e500 and now it's working perfectly. I will move on the other PRs. 😃

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.

3 participants