Skip to content

test_concurrent_futures.test_wait.ThreadPoolWaitTests.test_timeout flakes frequently on free-threaded Windows x64 #128364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
zanieb opened this issue Dec 30, 2024 · 5 comments
Labels
tests Tests in the Lib/test dir topic-free-threading type-bug An unexpected behavior, bug, or error

Comments

@zanieb
Copy link
Contributor

zanieb commented Dec 30, 2024

Bug report

Bug description:

The following test fails frequently, but not consistently:

test_timeout (test.test_concurrent_futures.test_wait.ThreadPoolWaitTests.test_timeout) ... FAIL
0.86s 
======================================================================
FAIL: test_timeout (test.test_concurrent_futures.test_wait.ThreadPoolWaitTests.test_timeout)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\cpython\cpython\Lib\test\test_concurrent_futures\test_wait.py", line 129, in test_timeout
    self.assertEqual(set([CANCELLED_AND_NOTIFIED_FUTURE,
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                          EXCEPTION_FUTURE,
                          ^^^^^^^^^^^^^^^^^
                          SUCCESSFUL_FUTURE]),
                          ^^^^^^^^^^^^^^^^^^^^
                     finished)
                     ^^^^^^^^^
AssertionError: Items in the second set but not the first:
<Future at 0x20000554620 state=finished returned NoneType>

As seen in the following CI runs for recent commits to main

A brief analysis of the code at

def test_timeout(self):
short_timeout = 0.050
long_timeout = short_timeout * 10
future = self.executor.submit(time.sleep, long_timeout)
finished, pending = futures.wait(
[CANCELLED_AND_NOTIFIED_FUTURE,
EXCEPTION_FUTURE,
SUCCESSFUL_FUTURE,
future],
timeout=short_timeout,
return_when=futures.ALL_COMPLETED)
self.assertEqual(set([CANCELLED_AND_NOTIFIED_FUTURE,
EXCEPTION_FUTURE,
SUCCESSFUL_FUTURE]),
finished)
self.assertEqual(set([future]), pending)

looks like the sleeping future is present in the finished set instead of pending

CPython versions tested on:

CPython main branch

Operating systems tested on:

Windows

Linked PRs

@zanieb zanieb added the type-bug An unexpected behavior, bug, or error label Dec 30, 2024
@zanieb
Copy link
Contributor Author

zanieb commented Dec 30, 2024

We could try bumping the sleep time (e.g., 2a150fc) but I don't think that would help. It's interesting it's consistently on this platform.

@colesbury
Copy link
Contributor

This might be a radical opinion, but I think we should delete the test. These sorts of timeout based tests that can spuriously fail if a process takes too long to start are a frequent source of flaky tests. I don't think they catch enough bugs or prevent enough regressions to justify the time we spend tweaking the tests.

@picnixz picnixz added the tests Tests in the Lib/test dir label Dec 31, 2024
@colesbury colesbury marked this as a duplicate of #130723 Mar 1, 2025
colesbury added a commit to colesbury/cpython that referenced this issue Mar 1, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 1, 2025
(cherry picked from commit cfa0b1d)

Co-authored-by: Sam Gross <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 1, 2025
(cherry picked from commit cfa0b1d)

Co-authored-by: Sam Gross <[email protected]>
@zanieb
Copy link
Contributor Author

zanieb commented Mar 1, 2025

Should be fixed by #130724 — can reopen if not.

@zanieb zanieb closed this as completed Mar 1, 2025
colesbury added a commit that referenced this issue Mar 1, 2025
gh-128364: Fix flaky `test_timeout` test (gh-130724)
(cherry picked from commit cfa0b1d)

Co-authored-by: Sam Gross <[email protected]>
colesbury added a commit that referenced this issue Mar 1, 2025
gh-128364: Fix flaky `test_timeout` test (gh-130724)
(cherry picked from commit cfa0b1d)

Co-authored-by: Sam Gross <[email protected]>
@picnixz
Copy link
Member

picnixz commented Mar 1, 2025

We have build bot failures on android and others now

@colesbury colesbury reopened this Mar 1, 2025
colesbury added a commit to colesbury/cpython that referenced this issue Mar 1, 2025
…)"

Change broke Android and iOS buildbots that do not have multiprocessing.

This reverts commit cfa0b1d.
colesbury added a commit that referenced this issue Mar 1, 2025
)

Change broke Android and iOS buildbots that do not have multiprocessing.

This reverts commit cfa0b1d.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 1, 2025
…)" (pythongh-130732)

Change broke Android and iOS buildbots that do not have multiprocessing.

This reverts commit cfa0b1d.
(cherry picked from commit 5221d9c)

Co-authored-by: Sam Gross <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 1, 2025
…)" (pythongh-130732)

Change broke Android and iOS buildbots that do not have multiprocessing.

This reverts commit cfa0b1d.
(cherry picked from commit 5221d9c)

Co-authored-by: Sam Gross <[email protected]>
colesbury added a commit that referenced this issue Mar 1, 2025
…h-130732) (#130735)

Revert "gh-128364: Fix flaky `test_timeout` test (gh-130724)" (gh-130732)

Change broke Android and iOS buildbots that do not have multiprocessing.

This reverts commit cfa0b1d.
(cherry picked from commit 5221d9c)

Co-authored-by: Sam Gross <[email protected]>
colesbury added a commit that referenced this issue Mar 1, 2025
…h-130732) (#130734)

Revert "gh-128364: Fix flaky `test_timeout` test (gh-130724)" (gh-130732)

Change broke Android and iOS buildbots that do not have multiprocessing.

This reverts commit cfa0b1d.
(cherry picked from commit 5221d9c)

Co-authored-by: Sam Gross <[email protected]>
colesbury added a commit to colesbury/cpython that referenced this issue Mar 1, 2025
Use events instead of relying on `time.sleep()`. The tests are also now about
four times faster.
colesbury added a commit to colesbury/cpython that referenced this issue Mar 6, 2025
colesbury added a commit that referenced this issue Mar 6, 2025
…0742)

Use events instead of relying on `time.sleep()`. The tests are also now about
four times faster.
colesbury added a commit to colesbury/cpython that referenced this issue Mar 6, 2025
… tests (pythongh-130742)

Use events instead of relying on `time.sleep()`. The tests are also now about
four times faster.
(cherry picked from commit c4d37ee)

Co-authored-by: Sam Gross <[email protected]>
colesbury added a commit that referenced this issue Mar 6, 2025
…gh-130742) (#130922)

Use events instead of relying on `time.sleep()`. The tests are also now about
four times faster.
(cherry picked from commit c4d37ee)
@colesbury
Copy link
Contributor

Should be fixed now

seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
…)" (pythongh-130732)

Change broke Android and iOS buildbots that do not have multiprocessing.

This reverts commit cfa0b1d.
seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
…ythongh-130742)

Use events instead of relying on `time.sleep()`. The tests are also now about
four times faster.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tests in the Lib/test dir topic-free-threading type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants