Skip to content

Timer set fixes #217

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

Merged
merged 5 commits into from
Jan 1, 2015
Merged

Timer set fixes #217

merged 5 commits into from
Jan 1, 2015

Conversation

rkday
Copy link
Contributor

@rkday rkday commented Jan 1, 2015

This fixes #216 and adds a UT for it. I also think I spotted a possible race condition (where we peek from a queue, do something with that task, and then pop from the queue, without guarding against the possibility that the head of the queue has changed since we peeked).

I checked that:

  • rake spec SPEC=spec/concurrent/executor/timer_set_spec.rb passes
  • rake spec SPEC=spec/concurrent/executor/timer_set_spec.rb fails without my commit 894ac23

@coveralls
Copy link

Coverage Status

Coverage decreased (-7.14%) when pulling af591e0 on rkday:timer_set_fixes into 4e7c5b8 on ruby-concurrency:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-7.04%) when pulling 960af94 on rkday:timer_set_fixes into 4e7c5b8 on ruby-concurrency:master.

@jdantonio
Copy link
Member

Thank you!

With respect to the failed Travis build, I've never seen that particular test fail on JRuby before. I don't believe it has anything to do with this patch so I've added it to the list of intermittently failing tests listed in Issue #117. We can look into it separate from this pull request.

jdantonio added a commit that referenced this pull request Jan 1, 2015
@jdantonio jdantonio merged commit 8ef62bb into ruby-concurrency:master Jan 1, 2015
@jdantonio
Copy link
Member

@rkday The failing JRuby tests seems to be a TravisCI issue. Our .travis.yml file specified jruby-19mode as the main JRuby build target. It appears that the fuzzy install that Travis initiates now installs jruby 9.0.0.0-SNAPSHOT for that directive. I've created PR #218 to fix.

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.

TimerSet doesn't pop any timers until the first timer pops
3 participants