[ruby-core:72386] [Ruby trunk - Bug #11822] Semantics of Queue#pop after close are wrong

[headius@headius.com]

Issue #11822 has been updated by Charles Nutter.


Summarizing some discussion with Koichi on #ruby-core:

* Current JRuby Queue uses Java's LinkedBlockingQueue and ArrayBlockingQueue, which both are locking implementations. There are lock-free queue implementations on JVM but they do not support blocking.
* Performance of the fully-locked impl of Queue from MRI 2.3 appears to be as good or better than JRuby's existing impls based on Java LBQ and ABQ for single and multi-threaded benchmarks. This is likely because the existing impls were also fully locked *and* JRuby had to implement num_waiters separately.
* The API proposed for #close, #push, and #pop for 2.3 is accepted by JRuby since there appears to be no performance implication right now.

So we should bump this to to Ruby 2.4 to address the inability to check for closed? and pop at the same time, as in Go. I believe that is the only remaining issue.

This can go forward in 2.3.

For reference, here's what OpenJDK lock-free queues are based upon: http://www.cs.rochester.edu/u/scott/papers/1996_PODC_queues.pdf

We may want to add a lock-free queue implementation to either Ruby or concurrent-ruby in the future.

----------------------------------------
Bug #11822: Semantics of Queue#pop after close are wrong
https://bugs.ruby-lang.org/issues/11822#change-55674

* Author: Charles Nutter
* Status: Open
* Priority: Normal
* Assignee: Koichi Sasada
* ruby -v: 
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
Current test/ruby/thread/test_queue.rb test_close has the following assertion that seems wrong to me:

```ruby
  def test_close
    [->{Queue.new}, ->{SizedQueue.new 3}].each do |qcreate|
      q = qcreate.call
      assert_equal false, q.closed?
      q << :something
      assert_equal q, q.close
      assert q.closed?
      assert_raise_with_message(ClosedQueueError, /closed/){q << :nothing}
      assert_equal q.pop, :something  # <<< THIS ONE
      assert_nil q.pop
      assert_nil q.pop
      # non-blocking
      assert_raise_with_message(ThreadError, /queue empty/){q.pop(non_block=true)}
    end
  end
```

Once a queue is closed, I don't think it should ever return a result anymore. The queue should be cleared and pop should always return nil.

In r52691, ko1 states that "deq'ing on closed queue returns nil, always." This test does not match that behavior.



-- 
https://bugs.ruby-lang.org/