https://bugs.ruby-lang.org/https://bugs.ruby-lang.org/favicon.ico?17113305112022-08-29T10:21:21ZRuby Issue Tracking SystemRuby master - Feature #18982: Add an `exception: false` argument for Queue#push, Queue#pop, SizedQueue#push and SizedQueue#pophttps://bugs.ruby-lang.org/issues/18982?journal_id=989952022-08-29T10:21:21Zbyroot (Jean Boussier)byroot@ruby-lang.org
<ul><li><strong>Related to</strong> <i><a class="issue tracker-2 status-6 priority-4 priority-default closed" href="/issues/18965">Feature #18965</a>: Further Thread::Queue improvements</i> added</li></ul> Ruby master - Feature #18982: Add an `exception: false` argument for Queue#push, Queue#pop, SizedQueue#push and SizedQueue#pophttps://bugs.ruby-lang.org/issues/18982?journal_id=989962022-08-29T10:23:29Zbyroot (Jean Boussier)byroot@ruby-lang.org
<ul></ul><p>Proposed patch: <a href="https://github.com/ruby/ruby/pull/6299" class="external">https://github.com/ruby/ruby/pull/6299</a></p> Ruby master - Feature #18982: Add an `exception: false` argument for Queue#push, Queue#pop, SizedQueue#push and SizedQueue#pophttps://bugs.ruby-lang.org/issues/18982?journal_id=989972022-08-29T10:24:16ZEregon (Benoit Daloze)
<ul></ul><p>Sounds good :+1:</p> Ruby master - Feature #18982: Add an `exception: false` argument for Queue#push, Queue#pop, SizedQueue#push and SizedQueue#pophttps://bugs.ruby-lang.org/issues/18982?journal_id=990142022-08-30T00:08:09Zshyouhei (Shyouhei Urabe)shyouhei@ruby-lang.org
<ul></ul><p>+1 for avoiding exceptions but <code>nil</code> can be problematic? Because a closed queue would also return <code>nil</code> for <code>pop</code>. You cannot distinguish if a queue is closed or would just block.</p> Ruby master - Feature #18982: Add an `exception: false` argument for Queue#push, Queue#pop, SizedQueue#push and SizedQueue#pophttps://bugs.ruby-lang.org/issues/18982?journal_id=990152022-08-30T00:15:02Zbyroot (Jean Boussier)byroot@ruby-lang.org
<ul></ul><blockquote>
<p>but nil can be problematic?</p>
</blockquote>
<p>This was discussed in <a class="issue tracker-2 status-5 priority-4 priority-default closed" title="Feature: Add Queue#pop(timeout:) (Closed)" href="https://bugs.ruby-lang.org/issues/18774">#18774</a>:</p>
<ul>
<li>
<code>#pop</code> on closed queue already returns <code>nil</code> with no exception. <code>Queue.new.tap(&:close).pop # => nil</code>
</li>
<li>
<code>nil</code> make sense because you can use it in a loop such as <code>while item = queue.pop</code>.</li>
<li>There's not really any other value that make sense.</li>
</ul>
<p>Overall it's really not that complicated to handle the various cases in which nil is returned. I have an real world example here: <a href="https://github.com/Shopify/statsd-instrument/blob/0af8a1e2e74fc24d1de8088ee995940033c1fe42/lib/statsd/instrument/batched_udp_sink.rb#L111-L138" class="external">https://github.com/Shopify/statsd-instrument/blob/0af8a1e2e74fc24d1de8088ee995940033c1fe42/lib/statsd/instrument/batched_udp_sink.rb#L111-L138</a>.</p> Ruby master - Feature #18982: Add an `exception: false` argument for Queue#push, Queue#pop, SizedQueue#push and SizedQueue#pophttps://bugs.ruby-lang.org/issues/18982?journal_id=990162022-08-30T00:42:39Zjeremyevans0 (Jeremy Evans)merch-redmine@jeremyevans.net
<ul></ul><p>shyouhei (Shyouhei Urabe) wrote in <a href="#note-4">#note-4</a>:</p>
<blockquote>
<p>+1 for avoiding exceptions but <code>nil</code> can be problematic? Because a closed queue would also return <code>nil</code> for <code>pop</code>. You cannot distinguish if a queue is closed or would just block.</p>
</blockquote>
<p>We could add another keyword argument for the exception value, and have that value returned instead of raising an exception (the keyword argument would default to <code>nil</code>). I'm not sure if it's worth supporting that, but it is a simple approach.</p> Ruby master - Feature #18982: Add an `exception: false` argument for Queue#push, Queue#pop, SizedQueue#push and SizedQueue#pophttps://bugs.ruby-lang.org/issues/18982?journal_id=990222022-08-30T08:36:50Zshyouhei (Shyouhei Urabe)shyouhei@ruby-lang.org
<ul></ul><p>I don't think people want to exit from a <code>while item = queue.pop</code> loop because the queue would block. <code>for (;;) { if (errno != EAGAIN) break; ... }</code> is a C idiom (people often break from a loop on error, <em>except</em> EAGAIN).</p> Ruby master - Feature #18982: Add an `exception: false` argument for Queue#push, Queue#pop, SizedQueue#push and SizedQueue#pophttps://bugs.ruby-lang.org/issues/18982?journal_id=992382022-09-22T04:50:10Zko1 (Koichi Sasada)
<ul></ul><p><code>Queue#pop(timeout:0)</code> returns <code>nil</code> if Queue is empty. Is it enough? (and forget optional nonblock argument)</p> Ruby master - Feature #18982: Add an `exception: false` argument for Queue#push, Queue#pop, SizedQueue#push and SizedQueue#pophttps://bugs.ruby-lang.org/issues/18982?journal_id=992402022-09-22T06:46:03Zbyroot (Jean Boussier)byroot@ruby-lang.org
<ul></ul><blockquote>
<p>Is it enough?</p>
</blockquote>
<p>Interesting, I never thought of that.</p>
<p>I think I can work with that, but then I think this behavior should be documented and speced.</p>
<p>Because <code>timeout: 0</code> in some APIs (not necessarily ruby) can mean no timeout.</p> Ruby master - Feature #18982: Add an `exception: false` argument for Queue#push, Queue#pop, SizedQueue#push and SizedQueue#pophttps://bugs.ruby-lang.org/issues/18982?journal_id=994812022-10-06T06:47:14Zko1 (Koichi Sasada)
<ul></ul><blockquote>
<p>Because timeout: 0 in some APIs (not necessarily ruby) can mean no timeout.</p>
</blockquote>
<p>Already the implementation do -> "Queue#pop(timeout:0) returns nil if Queue is empty" so no problem?</p> Ruby master - Feature #18982: Add an `exception: false` argument for Queue#push, Queue#pop, SizedQueue#push and SizedQueue#pophttps://bugs.ruby-lang.org/issues/18982?journal_id=994822022-10-06T06:49:22Zbyroot (Jean Boussier)byroot@ruby-lang.org
<ul></ul><p>Works for me. I'll leave this ticket open until I add some more spec and documentation to the existing method.</p> Ruby master - Feature #18982: Add an `exception: false` argument for Queue#push, Queue#pop, SizedQueue#push and SizedQueue#pophttps://bugs.ruby-lang.org/issues/18982?journal_id=996592022-10-17T14:56:20Zbyroot (Jean Boussier)byroot@ruby-lang.org
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Closed</i></li></ul><p>Applied in changeset <a class="changeset" title="thread_sync.c: Clarify and document the behavior of timeout == 0 [Feature #18982] Instead of in..." href="https://bugs.ruby-lang.org/projects/ruby-master/repository/git/revisions/60defe0a68a40d1b3225cf6b971ea195e19ae2e2">git|60defe0a68a40d1b3225cf6b971ea195e19ae2e2</a>.</p>
<hr>
<p>thread_sync.c: Clarify and document the behavior of timeout == 0</p>
<p>[Feature <a class="issue tracker-2 status-5 priority-4 priority-default closed" title="Feature: Add an `exception: false` argument for Queue#push, Queue#pop, SizedQueue#push and SizedQueue#pop (Closed)" href="https://bugs.ruby-lang.org/issues/18982">#18982</a>]</p>
<p>Instead of introducing an <code>exception: false</code> argument to have <code>non_block</code><br>
return nil rather than raise, we can clearly document that a timeout of 0<br>
immediately returns.</p>
<p>The code is refactored a bit to avoid doing a time calculation in<br>
such case.</p>