ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
From: email@pitr.ch
To: ruby-core@ruby-lang.org
Subject: [ruby-core:72380] [Ruby trunk - Bug #11822] Semantics of Queue#pop after close are wrong
Date: Sat, 19 Dec 2015 13:37:23 +0000	[thread overview]
Message-ID: <redmine.journal-55670.20151219133721.4e551014b63ffa27@ruby-lang.org> (raw)
In-Reply-To: redmine.issue-11822.20151215160530@ruby-lang.org

Issue #11822 has been updated by Petr Chalupa.


Thank you for explanation. So nil can mean a normal nil pushed to the queue or closed queue, that might be error prone. As Charles mentioned there are many possible behaviors users might want when queue is closed. I would suggest to make this configurable on queue initialization. 

~~~ ruby
Queue.new on_close: nil # pops nil
Queue.new on_close: :poison_pill # pops symbol :poison_pill
CLOSED = Object.new
Queue.new on_close: CLOSED # pops CLOSED
Queue.new on_close: ClosedQueueError # raises exception of supplied class
~~~

Raising exception seems unnecessary to me, since close is normal feature of the queue not an exceptional state (this is subjective though so including in the example too).

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

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

  parent reply	other threads:[~2015-12-19 13:05 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <redmine.issue-11822.20151215160530@ruby-lang.org>
2015-12-15 16:05 ` [ruby-core:72149] [Ruby trunk - Bug #11822] [Open] Semantics of Queue#pop after close are wrong headius
2015-12-15 16:58 ` [ruby-core:72150] [Ruby trunk - Bug #11822] " headius
2015-12-15 17:48 ` [ruby-core:72152] " headius
2015-12-15 17:52 ` [ruby-core:72155] " headius
2015-12-15 23:47 ` [ruby-core:72164] " ko1
2015-12-16  7:39 ` [ruby-core:72178] " funny.falcon
2015-12-18  5:32 ` [ruby-core:72357] " ko1
2015-12-18 18:41 ` [ruby-core:72369] " email
2015-12-19  1:39 ` [ruby-core:72372] " ko1
2015-12-19 13:37 ` email [this message]
2015-12-19 14:37 ` [ruby-core:72382] " headius
2015-12-19 15:01 ` [ruby-core:72383] " email
2015-12-19 15:27 ` [ruby-core:72386] " headius
2015-12-19 15:29 ` [ruby-core:72387] " headius
2015-12-20  8:54 ` [ruby-core:72408] " funny.falcon
2016-01-25 18:21   ` [ruby-core:73429] " Andrew Vit
2017-01-31  7:12 ` [ruby-core:79334] [Ruby trunk Bug#11822][Closed] " ko1

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.ruby-lang.org/en/community/mailing-lists/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=redmine.journal-55670.20151219133721.4e551014b63ffa27@ruby-lang.org \
    --to=ruby-core@ruby-lang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).