classification
Title: multiprocessing.Queue.get(block=True, timeout=0) always raises queue.Empty
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: altuezi, davin, josh.r, rhettinger
Priority: normal Keywords:

Created on 2016-12-15 20:44 by altuezi, last changed 2016-12-18 06:48 by serhiy.storchaka.

Pull Requests
URL Status Linked Edit
PR 51 altuezi, 2016-12-15 20:44
Messages (5)
msg283344 - (view) Author: Ryan Brindley (altuezi) * Date: 2016-12-15 20:44
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
msg283357 - (view) Author: Josh Rosenberg (josh.r) * (Python triager) Date: 2016-12-16 01:09
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.
msg283422 - (view) Author: Ryan Brindley (altuezi) * Date: 2016-12-16 17:27
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.
msg283432 - (view) Author: Ryan Brindley (altuezi) * Date: 2016-12-16 20:21
In addition, queue.Queue supports timeout value of 0 and its documentation says "non-negative number".
msg283541 - (view) Author: Ryan Brindley (altuezi) * Date: 2016-12-18 06:30
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).
History
Date User Action Args
2016-12-18 06:48:13serhiy.storchakasetnosy: + rhettinger
2016-12-18 06:30:48altuezisetmessages: + msg283541
2016-12-16 20:21:19altuezisetmessages: + msg283432
2016-12-16 17:27:25altuezisetmessages: + msg283422
2016-12-16 01:09:01josh.rsetnosy: + josh.r
messages: + msg283357
2016-12-15 20:48:11ned.deilysetnosy: + davin
2016-12-15 20:44:25altuezicreate