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

[ko1@atdot.net]

Issue #11822 has been updated by Koichi Sasada.

Assignee set to Koichi Sasada

# Semantics

I'm not sure all I can understand, but Queue#close does not remove remaining items as Yura said.

(on 2.3) Queue#close is only for simple way for the following code:

```ruby
# without Queue#close

q = Queue.new
3.times{
  Thread.new{ # workers
    while e = q.pop
      work_with(e)
    end
  }
}

q.push work1
q.push work2
q.push work3
q.push work4
3.times{
  q.push nil
}
```

```ruby
# with Queue#close

q = Queue.new
3.times{
  Thread.new{ # workers
    while e = q.pop
      work_with(e)
    end
  }
}

q.push work1
q.push work2
q.push work3
q.push work4
q.close # simplified
```

Suggestions about documentation are welcome.

# Atomicity

I think it is possible to implement in atomic with appropriate locks. For example, we can make this implementation on pthread libraries.
However, such locks can be affect performance, especially on well-optimized lock-free queue implementations such as Java has.

I can't decide the problem is critical or not.
We have no time for Ruby 2.3 to prove it.

So I can remove Queue#close and continue discussion for ruby 2.4 if someone (Charles) says it should be removed.

Charles, what do you think?


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

* 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/