| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Provides a way to forcefully stop all the workers in the pool Typically this would be used as a last effort to stop all workers if unable to shutdown / join in the expected way
Sorry, something went wrong.
There was a problem hiding this comment.
Some additional comments. It would be great to have type checks test, e.g., when you pass an invalid signal value (namely, check that os.kill would raise a TypeError / ValueError).
Sorry, something went wrong.
|
Thanks @picnixz, I think I resolved all of your comments, please check again when you can. |
Sorry, something went wrong.
|
Hey @gpshead mind taking a look? Ref https://discuss.python.org/t/cancel-running-work-in-processpoolexecutor/58605/2 |
Sorry, something went wrong.
There was a problem hiding this comment.
Some other comments. I'll let @gpshead take over the review for a more in-depth knowledge (I'm not enough well-versed in concurrent.futures since I mainly use multiprocessing instead).
Sorry, something went wrong.
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
@csm10495 If you want the CI to be re-run, don't hesitate to ping me. |
Sorry, something went wrong.
|
@gpshead mind taking a look? |
Sorry, something went wrong.
|
Can I help to push the PR forward? This would be a very useful feature. |
Sorry, something went wrong.
|
I'm not confident enough with the subtlties of multiprocessing to merge the feature myself. @gpshead Can you have a look at this one please? |
Sorry, something went wrong.
|
@picnixz, I've done all the suggestions except swapping parameterize for individual methods. I genuinely think the current format is easier to understand for the bulk of folks looking at the code. For now terminate vs kill has more/less the same behavior for most users (the only difference being the signal on non win32). I understand splitting it out if the functionality plays out differently, but for now its all more/less the same, so I don't see the reason to split it out that much. All that being said, if you insist on it: I can make the changes. Just wanted to give one small plea for this way instead. |
Sorry, something went wrong.
There was a problem hiding this comment.
The rationale behind my suggestion is twofold:
However, I think using parametrize is fine now. I think what bothered me most was _terminate_or_kill naming because. But now it looks better. I would still want you to respect 80 chars wrap.
Sorry, something went wrong.
|
I'll merge this one tomorrow or Gregory can merge it sooner if he wants (I usually avoid merging PRs after 8 PM as I try to have a nice commit message which is harder to find when I'm tired) |
Sorry, something went wrong.
| else: | ||
| self.fail(f"Unknown operation: {function_name}") | ||
|
|
||
| self.assertRaises(queue.Empty, q.get, timeout=1) |
There was a problem hiding this comment.
I do suspect we may see this come up as occasionally flaky in buildbot or CI systems as it is depend on the timing of the sleeps and kills which really can't be guaranteed on a loaded system. if so, _put_sleep_put can have its sleep increased. lets see how it goes first.
Sorry, something went wrong.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️Hi! The buildbot aarch64 Android 3.x (tier-3) has failed when building commit f97e409. What do you need to do:
You can take a look at the buildbot page here: https://buildbot.python.org/#/builders/1594/builds/1441 Failed tests:
Failed subtests:
Summary of the results of the build (if available): == Click to see traceback logsTraceback (most recent call last):
File "/data/user/0/org.python.testbed/files/python/lib/python3.14/test/test_android.py", line 53, in setUp
self.assert_log("I", tag, message, skip=True, timeout=5)
~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/data/user/0/org.python.testbed/files/python/lib/python3.14/test/test_android.py", line 65, in assert_log
self.fail(f"line not found: {expected!r}")
~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: line not found: 'test.test_android.TestAndroidOutput.test_bytes 1740967680.0124214'
Traceback (most recent call last):
File "/data/user/0/org.python.testbed/files/python/lib/python3.14/test/test_android.py", line 63, in assert_log
line = self.logcat_queue.get(timeout=(deadline - time()))
File "/data/user/0/org.python.testbed/files/python/lib/python3.14/queue.py", line 212, in get
raise Empty
_queue.Empty
Traceback (most recent call last):
File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/runners.py", line 118, in run
return self._loop.run_until_complete(task)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/base_events.py", line 687, in run_until_complete
return future.result()
^^^^^^^^^^^^^^^
File "/Users/android/buildarea/3.x.mhsmith-android-aarch64/build/Android/android.py", line 531, in run_testbed
async with asyncio.TaskGroup() as tg:
File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/taskgroups.py", line 134, in __aexit__
raise propagate_cancellation_error
File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/taskgroups.py", line 110, in __aexit__
await self._on_completed_fut
asyncio.exceptions.CancelledError
|
Sorry, something went wrong.
|
The Android failure is #124666. Although it involves a queue, it doesn't involve concurrent.futures, so I don't think it's related to this PR. |
Sorry, something went wrong.
|
We have some build bots failures: https://buildbot.python.org/#/builders/568/builds/8398 but I don't know how important they are. |
Sorry, something went wrong.
|
We have other workers issues: https://github.com/python/cpython/actions/runs/13631446353/job/38099943353#step:11:686 |
Sorry, something went wrong.
|
It seems like on some platforms, we're not getting negative signals as the exit code on terminate/kill. Instead it gets a 255 which is odd. Though on a retry sometimes it passes, so it must be a race condition of some sort. When I get a moment I can try to swap from using signals to verify to patching with a side effect of running the actual method. |
Sorry, something went wrong.
|
I think we should first revert the commit for now because it's causing macOS builds to fail (for instance https://github.com/python/cpython/actions/runs/13636802329/job/38117531328?pr=121119) =/ |
Sorry, something went wrong.
|
Is there a way to get more info about what about the ENV changed in the test? I'm not seeing how/why this could cause that but knowing which key changed may help tell more. |
Sorry, something went wrong.
|
Sorry the real issue was: https://github.com/python/cpython/actions/runs/13636802329/job/38117531328?pr=121119#step:11:676. |
Sorry, something went wrong.
|
Wait. No, the issue is indeed the environment that changed. |
Sorry, something went wrong.
|
Yeah all i see is: 0:02:54 load avg: 3.52 [482/485/1] test.test_concurrent_futures.test_process_pool failed (env changed) -- running (1): test_ssl (31.5 sec) test_force_shutdown_workers (test.test_concurrent_futures.test_process_pool.ProcessPoolForkProcessPoolExecutorTest.test_force_shutdown_workers) ... /Users/admin/actions-runner/_work/cpython/cpython/Lib/multiprocessing/popen_fork.py:67: DeprecationWarning: This process (pid=9970) is multi-threaded, use of fork() may lead to deadlocks in the child. self.pid = os.fork() 2.22s ok |
Sorry, something went wrong.
|
Let's perhaps skip ignore this one which seems flaky. For instance it disappeared on my PR now. However, the buildbot failure is a true failure on some systems as it consistently fail on main |
Sorry, something went wrong.
|
I am still seeing test_concurrent_futures.test_process_pool CI failures in various PRs. I think we should revert this for now. |
Sorry, something went wrong.
…ethods to ProcessPoolExecutor (pythonGH-128043)" The test_concurrent_futures.test_process_pool test is failing in CI. This reverts commit f97e409.
| Back | FazBrowse Home | New Git URL |
Provides a way to forcefully stop all the workers in the pool
Typically this would be used as a last effort to stop all workers if unable to shutdown / join in the expected way.