classification
Title: test_concurrent_futures.test_crash() failed on x86 Windows7 3.7
Type: Stage: resolved
Components: Tests, Windows Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: miss-islington, pablogsal, paul.moore, pitrou, steve.dower, tim.golden, vstinner, zach.ware
Priority: normal Keywords: patch

Created on 2018-05-31 12:41 by vstinner, last changed 2018-07-12 09:12 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 7828 merged pablogsal, 2018-06-20 19:40
PR 8263 merged miss-islington, 2018-07-12 08:46
PR 8264 merged vstinner, 2018-07-12 08:52
Messages (9)
msg318294 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-05-31 12:41
x86 Windows7 3.7:
http://buildbot.python.org/all/#/builders/111/builds/299

test_crash (test.test_concurrent_futures.ProcessPoolSpawnExecutorDeadlockTest) ... 26.57s ok
...
test_crash (test.test_concurrent_futures.ProcessPoolSpawnExecutorDeadlockTest) ... 90.96s FAIL
...

======================================================================
FAIL: test_crash (test.test_concurrent_futures.ProcessPoolSpawnExecutorDeadlockTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\cygwin\home\db3l\buildarea\3.7.bolen-windows7\build\lib\test\test_concurrent_futures.py", line 131, in tearDown
    self.assertLess(dt, 60, "synchronization issue: test lasted too long")
AssertionError: 90.95560574531555 not less than 60 : synchronization issue: test lasted too long


This buildbot is known to be slow.

See also bpo-33715.
msg320056 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-20 10:23
Hum, I guess that the fix is to use a timeout of 5 minutes instead of 1 minute. It's ok if the buildbot is slow.

Moreover, it would be interesting to replace time.time() with time.monotonic().
msg320074 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-20 12:35
Recent failure (fail then pass):
http://buildbot.python.org/all/#/builders/58/builds/1031

======================================================================
FAIL: test_crash (test.test_concurrent_futures.ProcessPoolSpawnExecutorDeadlockTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\test\test_concurrent_futures.py", line 131, in tearDown
    self.assertLess(dt, 60, "synchronization issue: test lasted too long")
AssertionError: 62.650086402893066 not less than 60 : synchronization issue: test lasted too long
msg320126 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-20 23:00
+    def tearDown(self):
+        self.executor.shutdown(wait=True)
+        dt = time.time() - self.t1
+        if test.support.verbose:
+            print("%.2fs" % dt, end=' ')
+        self.assertLess(dt, 60, "synchronization issue: test lasted too long")

This code has been added by:

commit aebac0b55a1e3addb93ec7992046a4f9561b4175
Author: Antoine Pitrou <solipsis@pitrou.net>
Date:   Thu Mar 24 15:47:39 2011 +0100

    Add tests for the atexit hook in concurrent.futures (part of #11635)


What is the purpose of having an hardcoded maximum test execution duration? If a test takes 2 seconds instead of 1, it means that the test found a design issue in concurrent.futures? Or it would mean that the test has a bug?

We have many buildbots which are super slow, so I proposed to increase the maximum duration to 5 minutes instead of 1 minute to quickly repair buildbots.

But with 5 minutes, I'm not sure that the check is still useful. 

@Antoine: do you recall the rationale for this check?
msg320147 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2018-06-21 10:04
I don't remember :-/  It's probably ok to increase the timeout, though.
msg320169 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2018-06-21 11:30
New changeset 3ad8decd76c736f393755537aeb19b5612c21761 by Pablo Galindo in branch 'master':
bpo-33716, test_concurrent_futures: increase timeout (GH-7828)
https://github.com/python/cpython/commit/3ad8decd76c736f393755537aeb19b5612c21761
msg321530 - (view) Author: miss-islington (miss-islington) Date: 2018-07-12 09:06
New changeset b89776fb1b000f73a62850bea78e5b3434bd7e9a by Miss Islington (bot) in branch '3.7':
bpo-33716, test_concurrent_futures: increase timeout (GH-7828)
https://github.com/python/cpython/commit/b89776fb1b000f73a62850bea78e5b3434bd7e9a
msg321531 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-07-12 09:11
New changeset 8df4770e8d2c9ebd49c5e4d073eef3a5bc805cfc by Victor Stinner in branch '3.6':
bpo-33716, test_concurrent_futures: increase timeout (GH-7828) (GH-8264)
https://github.com/python/cpython/commit/8df4770e8d2c9ebd49c5e4d073eef3a5bc805cfc
msg321532 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-07-12 09:12
The issue has been working around by increasing the timeout from 1 minute to 5 minutes.
History
Date User Action Args
2018-07-12 09:12:17vstinnersetstatus: open -> closed
versions: + Python 3.6, Python 3.8
messages: + msg321532

resolution: fixed
stage: patch review -> resolved
2018-07-12 09:11:34vstinnersetmessages: + msg321531
2018-07-12 09:06:00miss-islingtonsetnosy: + miss-islington
messages: + msg321530
2018-07-12 08:52:49vstinnersetpull_requests: + pull_request7797
2018-07-12 08:46:53miss-islingtonsetpull_requests: + pull_request7796
2018-06-21 11:30:39pablogsalsetnosy: + pablogsal
messages: + msg320169
2018-06-21 10:04:52pitrousetmessages: + msg320147
2018-06-20 23:00:25vstinnersetnosy: + pitrou
messages: + msg320126
2018-06-20 19:40:17pablogsalsetkeywords: + patch
stage: patch review
pull_requests: + pull_request7436
2018-06-20 12:35:12vstinnersetmessages: + msg320074
2018-06-20 10:23:57vstinnersetmessages: + msg320056
2018-05-31 12:41:13vstinnercreate