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

Ruby 1.9 MonitorMixin::ConditionVariable#wait timeout not supported #8

Closed
geoffgarside opened this issue Dec 25, 2009 · 7 comments
Closed

Comments

@geoffgarside
Copy link
Contributor

Seems that presently the timeout parameter to the MonitorMixin::ConditionVariable#wait method is not supported in Ruby 1.9. This is currently used in lib/god/driver.rb:88. I'm not sure if the timeout used at this point in god is required or if its just adding a back-off so that the event isn't retried too quickly. It might be worth checking to see if god is being invoked by ruby 1.9 and then setting the delay to nil on that case.

@geoffgarside
Copy link
Contributor Author

I think I might have fixed this in my fork, but I can't get the unit tests to run to completion under Ruby 1.9 either with or without my changes.

@eric
Copy link
Collaborator

eric commented Feb 16, 2010

I've done a little to get the ConditionVariable working by grabbing code from the latest 1.9 changes:

http://github.com/eric/god/tree/condition-variable-1.9-support

@julian7
Copy link

julian7 commented Mar 27, 2010

I gave it a shot, and it seems to be working well on my laptop (osx, rvm, ruby 1.9.1). I'll let it run overnight on a server, let's see whether it works (freebsd, ruby 1.9.1 from ports).

@eric
Copy link
Collaborator

eric commented Mar 27, 2010

This is the same issue as #6.

@sometimesfood
Copy link

Your fixes for Ruby 1.9 seem to work nicely here (Debian Lenny, Ruby 1.9.0).

Is there anything specific you'd like to have tested that we can help with?

@mojombo
Copy link
Owner

mojombo commented Apr 4, 2010

Dupe of #6. The monitor patches are in 0.9.0.

@julian7
Copy link

julian7 commented Apr 4, 2010

Thanks for the update. You may close this issue.

This issue was closed.
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

5 participants