Project

General

Profile

Actions

Feature #9145

open

Queue#pop(true) return nil if empty instead of raising ThreadError

Added by jsc (Justin Collins) almost 11 years ago. Updated almost 7 years ago.

Status:
Open
Assignee:
-
Target version:
-
[ruby-core:58545]

Description

I propose the non-blocking form of Queue#pop behave like Array#pop and return nil when empty.

Current behavior is to raise a ThreadError, with a message indicating the queue is empty.

For example:

q = Queue.new
begin
loop do
next_item = q.pop(true)
end
rescue ThreadError

queue is empty...or maybe something bad happened

end

Instead, this could be

q = Queue.new
while next_item = q.pop(true)
end

Alternatively, raise an exception that is a subclass of ThreadError with a more specific name, such as "QueueEmpty". This would be a small improvement while remaining compatible with existing code.

Updated by Glass_saga (Masaki Matsushita) almost 11 years ago

  • Category changed from lib to ext
  • Status changed from Open to Feedback

I think we can't change default behavior of Queue#pop(true) because some code expects ThreadError to be raised.
However, it may be possible to introduce new keyword argument like following:

q = Queue.new
while next_item = q.pop(true, exception: false) # it doesn't raise ThreadError and returns nil.

do something

end

Updated by normalperson (Eric Wong) almost 11 years ago

"Glass_saga (Masaki Matsushita)" wrote:

I think we can't change default behavior of Queue#pop(true) because some code expects ThreadError to be raised.
However, it may be possible to introduce new keyword argument like following:

q = Queue.new
while next_item = q.pop(true, exception: false) # it doesn't raise ThreadError and returns nil.

do something

end

+1 to that. All non-blocking methods (I/O or not) should support
exception: false to avoid (expensive/noisy-on-$DEBUG=$true) exceptions.

Updated by jsc (Justin Collins) almost 11 years ago

Glass_saga (Masaki Matsushita) wrote:

I think we can't change default behavior of Queue#pop(true) because some code expects ThreadError to be raised.
However, it may be possible to introduce new keyword argument like following:

q = Queue.new
while next_item = q.pop(true, exception: false) # it doesn't raise ThreadError and returns nil.

do something

end

That would work for me.

Updated by Anonymous almost 11 years ago

On 11/23/2013 08:30 PM, Glass_saga (Masaki Matsushita) wrote:

I think we can't change default behavior of Queue#pop(true) because some code expects ThreadError to be raised.
However, it may be possible to introduce new keyword argument like following:

q = Queue.new
while next_item = q.pop(true, exception: false) # it doesn't raise ThreadError and returns nil.

do something

end

Or what about a new method, Queue#pop?, which is always non-blocking and
non-raising. It would behave like:

class Queue
def pop?
pop(true)
rescue ThreadError
nil
end
end

q = Queue.new

q << 1
q << 2
q << 3

while x=q.pop?
p x
end

END
output:
1
2
3

Updated by drbrain (Eric Hodel) almost 11 years ago

Note that the current behavior allows you to distinguish between a nil in the queue (returns nil) and no value in the queue (raises ThreadError)

Updated by Glass_saga (Masaki Matsushita) over 7 years ago

  • Status changed from Feedback to Closed

Currently, Queue#pop takes non_block flag.

Updated by normalperson (Eric Wong) over 7 years ago

wrote:

Issue #9145 has been updated by Glass_saga (Masaki Matsushita).

Status changed from Feedback to Closed

Currently, Queue#pop takes non_block flag.

No, I don't think this should be closed.

I think Justin's point was:

Currently, it is impossible to know if a queue is closed
(permanent condition) or if it is empty (temporary condition).
So at the very least, a different exception should be raised:

Justin Collins wrote:
> Alternatively, raise an exception that is a subclass of
> ThreadError with a more specific name, such as "QueueEmpty".
> This would be a small improvement while remaining compatible
> with existing code.

On a side note, relying on exceptions for flow control has all
the same performance and $DEBUG noise problems it did with
IO#*_nonblock [ruby-core:38666] [Feature #5138]

But thinking of an efficient API for that is tricky :<

Actions #8

Updated by Glass_saga (Masaki Matsushita) over 7 years ago

  • Status changed from Closed to Open

Updated by uwe@kubosch.no (Uwe Kubosch) almost 7 years ago

How about a block form where the block is called with the popped element? The method would return false if called with non_block set to true if the queue is empty.

q = Queue.new
q << 1
q << 2
q << 3

while q.pop(true){|x| p x}

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0