Issue 30595: test_queue_feeder_donot_stop_onexc() of test_multiprocessing_spawn fails randomly on x86 Windows7 3.x

http://buildbot.python.org/all/builders/x86%20Windows7%203.x/builds/712/steps/test/logs/stdio

======================================================================
ERROR: test_queue_feeder_donot_stop_onexc (test.test_multiprocessing_spawn.WithProcessesTestQueue)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\test\_test_multiprocessing.py", line 767, in test_queue_feeder_donot_stop_onexc
    self.assertTrue(q.get(timeout=0.1))
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\multiprocessing\queues.py", line 105, in get
    raise Empty
queue.Empty

Hum, it may be a regression a recent change like the commit bc50f03db4f58c869b78e98468e374d7e61f1227:
    bpo-30414: multiprocessing.Queue._feed do not break from main loop on exc (#1683)

Looks like build bot is too slow for timeout=0.1. 

I am guessing `0.1` is too low because we have wrong condition in Queue.get.

It should be.

diff --git a/Lib/multiprocessing/queues.py b/Lib/multiprocessing/queues.py
index dda03dd..42e9884 100644
--- a/Lib/multiprocessing/queues.py
+++ b/Lib/multiprocessing/queues.py
@@ -101,7 +101,7 @@ class Queue(object):
             try:
                 if block:
                     timeout = deadline - time.time()
-                    if timeout < 0 or not self._poll(timeout):
+                    if self._poll(timeout):
                         raise Empty
                 elif not self._poll():
                     raise Empty

If we successfully acquired self._rlock, we should poll no matter how long acquire took.

of course it should be `if not`:

diff --git a/Lib/multiprocessing/queues.py b/Lib/multiprocessing/queues.py
index dda03dd..514f991 100644
--- a/Lib/multiprocessing/queues.py
+++ b/Lib/multiprocessing/queues.py
@@ -101,7 +101,7 @@ class Queue(object):
             try:
                 if block:
                     timeout = deadline - time.time()
-                    if timeout < 0 or not self._poll(timeout):
+                    if not self._poll(timeout):
                         raise Empty
                 elif not self._poll():
                     raise Empty

> I am guessing `0.1` is too low because we have wrong condition in Queue.get. It should be.

I'm not sure that the buildbot took this code path, but I like your suggestion. Checking the reader in non-blocking mode is cheap.

poll() with timeout < 0 works as poll(0), I checked the UNIX and Windows implementations.

So I proposed your change as a PR, and credited you ;-)
https://github.com/python/cpython/pull/2027

New changeset 1b7863c3b6519c6e134c28cab8b8af0dea43e375 by Victor Stinner in branch 'master':
bpo-30595: Fix multiprocessing.Queue.get(timeout) (#2027)
https://github.com/python/cpython/commit/1b7863c3b6519c6e134c28cab8b8af0dea43e375

Thanks Grzegorz Grzywacz! I commited your idea of fix. Let's if it fixes the issue. It will be hard to check since it seems like the bug was random.

Later, I will backport the fix to other branches.

Let's *see* if it fixes the issue. It will be hard to check since it seems
like the bug was random.

> New changeset 1b7863c3b6519c6e134c28cab8b8af0dea43e375 by Victor Stinner in branch 'master':
> bpo-30595: Fix multiprocessing.Queue.get(timeout) (#2027)
> https://github.com/python/cpython/commit/1b7863c3b6519c6e134c28cab8b8af0dea43e375

While this change is a nice fix, it isn't enough to make the test stable on slow buildbot: it failed again on the same buildbot,
http://buildbot.python.org/all/builders/x86%20Windows7%203.x/builds/744

If I modify the timeout from 100 ms to 1 ms, the test also fail on Linux on my laptop. So I proposed https://github.com/python/cpython/pull/2148 to increase the timeout from 100 ms to 1 sec.

New changeset 8f6eeaf21cdf4aea25fdefeec814a1ce07453fe9 by Victor Stinner in branch 'master':
bpo-30595: Increase test_queue_feeder_donot_stop_onexc() timeout (#2148)
https://github.com/python/cpython/commit/8f6eeaf21cdf4aea25fdefeec814a1ce07453fe9

New changeset e42339d3a08a8fde3349722def85d7a8e49899be by Victor Stinner in branch '3.6':
[3.6] bpo-30595: Fix multiprocessing.Queue.get(timeout) (#2027) (#2881)
https://github.com/python/cpython/commit/e42339d3a08a8fde3349722def85d7a8e49899be

New changeset ec9a7127b8196dc5ef27c5ee2772ecea7cbb32df by Victor Stinner in branch '2.7':
[3.6] bpo-30595: Fix multiprocessing.Queue.get(timeout) (#2027) (#2881) (#2883)
https://github.com/python/cpython/commit/ec9a7127b8196dc5ef27c5ee2772ecea7cbb32df

I fixed multiprocessing.Queue.get(timeout) in 2.7, 3.6 and master. I close the issue.

New changeset b60f43a0e6052b29517931eea591b0b29903f382 by Victor Stinner in branch '2.7':
bpo-30595: Increase test_queue_feeder_donot_stop_onexc() timeout (GH-2148) (GH-5429)
https://github.com/python/cpython/commit/b60f43a0e6052b29517931eea591b0b29903f382

The Python 2.7 is to fix this failure:
http://buildbot.python.org/all/#/builders/59/builds/61

test_lock (test.test_multiprocessing.TestForkAwareThreadLock) ... ok
test_ignore (test.test_multiprocessing.TestIgnoreEINTR) ... ok
test test_multiprocessing failed -- Traceback (most recent call last):
  File "/Users/db3l/buildarea/2.7.bolen-tiger/build/Lib/test/test_multiprocessing.py", line 674, in test_queue_feeder_donot_stop_onexc
    self.assertTrue(q.get(timeout=0.1))
  File "/Users/db3l/buildarea/2.7.bolen-tiger/build/Lib/multiprocessing/queues.py", line 132, in get
    raise Empty
Empty
test_ignore_listener (test.test_multiprocessing.TestIgnoreEINTR) ... ok
======================================================================
ERROR: test_queue_feeder_donot_stop_onexc (test.test_multiprocessing.WithProcessesTestQueue)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/db3l/buildarea/2.7.bolen-tiger/build/Lib/test/test_multiprocessing.py", line 674, in test_queue_feeder_donot_stop_onexc
    self.assertTrue(q.get(timeout=0.1))
  File "/Users/db3l/buildarea/2.7.bolen-tiger/build/Lib/multiprocessing/queues.py", line 132, in get
    raise Empty
Empty

resolution: fixed
stage: resolved