multiprocessing.Queue.get(block=True, timeout=0) always raises queue.Empty

[straticsryan]

BPO	28982
Nosy	@rhettinger, @MojoVampire, @applio, @straticsryan, @iritkatriel
PRs	bpo-28598: Support __rmod__ for RHS subclasses of str in % string formatting operations #51

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2022-01-29.13:05:19.253>
created_at = <Date 2016-12-15.20:44:25.736>
labels = ['3.7', 'type-bug', 'library']
title = 'multiprocessing.Queue.get(block=True, timeout=0) always raises queue.Empty'
updated_at = <Date 2022-01-29.13:05:19.252>
user = 'https://github.com/straticsryan'

bugs.python.org fields:

activity = <Date 2022-01-29.13:05:19.252>
actor = 'iritkatriel'
assignee = 'none'
closed = True
closed_date = <Date 2022-01-29.13:05:19.253>
closer = 'iritkatriel'
components = ['Library (Lib)']
creation = <Date 2016-12-15.20:44:25.736>
creator = 'altuezi'
dependencies = []
files = []
hgrepos = []
issue_num = 28982
keywords = []
message_count = 6.0
messages = ['283344', '283357', '283422', '283432', '283541', '411058']
nosy_count = 5.0
nosy_names = ['rhettinger', 'josh.r', 'davin', 'altuezi', 'iritkatriel']
pr_nums = ['51']
priority = 'normal'
resolution = 'out of date'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue28982'
versions = ['Python 3.7']

[straticsryan]

Hey dev team,

According to the following test, q.get(True, 0) always raises queue.Empty.

from multiprocessing import Queue
q = Queue()
q.put('foo')
q.get(True, 0)  # raises Empty

This result throws me off as I was expecting a similar result to the underlying poll/select timeout where 0 actually just means non-blocking.

After reviewing Lib/multiprocessing/queues.py, I found the condition where the timeout, after the adjustment for the time required to acquire the lock, would not even call poll() if it was less than 0.

So, linked is a simple PR that I'm offering as a suggested fix/behavior-change of Queue.get's handling of timeout=0 to match the underlying poll/select handling (aka non-blocking).

Cheers,

Ryan Brindley

[MojoVampire]

That argument combination appears to be undefined in the docs, the only cases covered are:

block truthy, timeout is None

block truthy, timeout is positive

block falsy, (timeout unspecified)

The case of block truthy, timeout <= 0 is not documented.

Saying "Block for a result, but not for any length of time" is a tad strange; I'd be inclined to make it an error, not a synonym for block == False.

[straticsryan]

So, the code handles timeout = 0 on systems where time.time() returns an int. Look at the following snippet and consider 2 assumptions: (1) time.time() returns an int, and (2) self._rlock.acquire call takes less than a second

            if block:
                deadline = time.time() + timeout
            if not self._rlock.acquire(block, timeout):
                raise Empty
            try:
                if block:
                    timeout = deadline - time.time()
                    if timeout < 0 or not self._poll(timeout):
                        raise Empty

The problem for the timeout = 0 case happens on systems where time.time() returns a floating point number and the acquire lock takes enough time to cause a diff in time.time() result between the deadline instantiation line and the timeout update line.

Given that, especially the if timeout < 0 ... line, I thought it may have been in the original intent for the function to handle 0 timeout when block truthy.

That side, the whole concept of having a separate block bool arg in the first place is a tad strange for me. Isn't it a bit redundant?

Why not just follow the underlying poll/select timeout arg logic as follows:

timeout = None, block indefinitely
timeout <= 0, non-block
timeout > 0, block timeout length

We could simplify this interaction by just removing that block arg all together. Not that I'm asking to do that here though, but maybe in the future?

If we go down the suggested error-raising path though, I would ask that the error not be queue.Empty -- which is misleading.

[straticsryan]

In addition, queue.Queue supports timeout value of 0 and its documentation says "non-negative number".

[straticsryan]

I've updated the PR to also include raising a ValueError for timeout values < 0.

This behavior mimics that of queue.Queue (noting here again that queue.Queue handles timeout = 0).

[iritkatriel]

On 3.11 I get this on both Mac and windows:

>>> from multiprocessing import Queue
>>> q = Queue()
>>> q.put('foo')
>>> q.get(True, 0)  # raises Empty
'foo'