Skip to content

Fix SizedQueue#num_waiting regression#3597

Merged
headius merged 1 commit intojruby:masterfrom
etehtsea:num_waiting_regression
Jan 19, 2016
Merged

Fix SizedQueue#num_waiting regression#3597
headius merged 1 commit intojruby:masterfrom
etehtsea:num_waiting_regression

Conversation

@etehtsea
Copy link
Contributor

@etehtsea etehtsea commented Jan 12, 2016

Introduced in d13405b.

JRuby 1.7 and MRI 2.2.3:

>> sq = SizedQueue.new(1)
=> #<SizedQueue:0x61f8bee4>
>> sq.push(1)
=> nil
>> Thread.new { sq.push(2) }
=> #<Thread:0x7fac631b run>
>> sq.num_waiting
=> 1

JRuby 9.0.4.0:

>> sq = SizedQueue.new(1)
=> #<SizedQueue:0x103f852>
>> sq.push(1)
=> #<SizedQueue:0x103f852>
>> Thread.new { sq.push(2) }
=> #<Thread:0x4ae82894 sleep>
>> sq.num_waiting
=> 0

P.S. I'm not specialized in Java, but it seems to work.

@etehtsea etehtsea changed the title Add spec for SizedQueue#num_waiting regression Fix SizedQueue#num_waiting regression Jan 12, 2016
@etehtsea etehtsea force-pushed the num_waiting_regression branch from 7824b91 to 892592b Compare January 12, 2016 12:31
@eregon
Copy link
Member

eregon commented Jan 12, 2016

Thanks for the nice spec!

@thedarkone
Copy link
Contributor

thedarkone commented Jan 14, 2016

I think this is also fixed on ruby-2.3 branch (with the new queue).

@etehtsea
Copy link
Contributor Author

etehtsea commented Jan 19, 2016

@thedarkone

~/Projects/jruby [ ruby-2.3: ✗ ] 7d ⚡ bin/jruby spec/mspec/bin/mspec run spec/ruby/library/thread/sizedqueue/num_waiting_spec.rb                                  2.2.3 ☺
jruby 9.0.5.0-SNAPSHOT (2.3.0) 2016-01-19 badd94b Java HotSpot(TM) 64-Bit Server VM 25.60-b23 on 1.8.0_60-b27 [darwin-x86_64]

1)
Thread::SizedQueue#num_waiting reports the number of threads waiting to push FAILED
Expected 0
 to equal 1

/Users/kes/Projects/jruby/spec/ruby/library/thread/sizedqueue/num_waiting_spec.rb:12:in `block in (root)'
org/jruby/RubyBasicObject.java:1633:in `instance_eval'
org/jruby/RubyEnumerable.java:1562:in `all?'
org/jruby/RubyFixnum.java:296:in `times'
org/jruby/RubyArray.java:1555:in `each'
/Users/kes/Projects/jruby/spec/ruby/library/thread/sizedqueue/num_waiting_spec.rb:5:in `<top>'
org/jruby/RubyKernel.java:957:in `load'
org/jruby/RubyBasicObject.java:1633:in `instance_eval'
org/jruby/RubyArray.java:1555:in `each'
[\ | ==================100%================== | 00:00:00]      1F      0E

Finished in 0.015000 seconds

1 file, 2 examples, 6 expectations, 1 failure, 0 errors, 0 tagged

Spec failed but I haven't made any further investigations yet.

UPD: Seems that failure is related to race-condition issue. When I added sleep 1 spec passed.

@kares kares added this to the JRuby 9.0.5.0 milestone Jan 19, 2016
@etehtsea etehtsea force-pushed the num_waiting_regression branch from 892592b to d54e4cd Compare January 19, 2016 09:25
@etehtsea
Copy link
Contributor Author

etehtsea commented Jan 19, 2016

I updated spec.

P.S. Hope it's safe to wait until thread fall asleep.

@kares
Copy link
Member

kares commented Jan 19, 2016

@thedarkone should be good to merge as is for JRuby 9.0.5 (non ruby-2.3), right?

@headius
Copy link
Member

headius commented Jan 19, 2016

I'll look at merging this today.

@headius
Copy link
Member

headius commented Jan 19, 2016

Note that https://github.com/spec/ruby has a different shared spec for num_waiting now, and JRuby's ruby-2.3 branch passes it fine: https://github.com/ruby/spec/blob/master/library/thread/shared/queue/num_waiting.rb

I'll merge this into master for 9.0.5.0 but I'll leave it up to you folks to decide which spec is better.

@headius headius merged commit d54e4cd into jruby:master Jan 19, 2016
@etehtsea
Copy link
Contributor Author

etehtsea commented Jan 19, 2016

@headius I don't see any difference from existing shared spec https://github.com/jruby/jruby/blob/master/spec/ruby/library/thread/shared/queue/num_waiting.rb

@etehtsea etehtsea deleted the num_waiting_regression branch January 19, 2016 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

JRuby 9.0.5.0

Development

Successfully merging this pull request may close these issues.

5 participants

@etehtsea @eregon @thedarkone @kares @headius