bpo-44336: Prevent hanging on child process handles #26578
Conversation
Replace the child process `typeperf.exe` with a daemon thread that reads the performance counters directly. This prevents the issues that arise from inherited handles in grandchild processes (see https://bugs.python.org/issue37531#msg350191).
|
Please add the test-with-buildbots label to ensure this actually fixes the buildbots! |
|
CC: @pablogsal |
|
Do note that it's currently failing on Windows CI, but I went ahead and added the label. |
|
It seems that buildbot runs were not sheduled for the Windows buildbots. :( Is it because they are paused (due to excessive exceptions?) or that the CI failed? |
|
I think it's just backlog, though I notice that your worker is currently "paused" (though I don't know why). |
|
Woohoo! The PR allows the buildbot test stage to finish successfully! Well, in this case, with an error, but it no longer hangs waiting for the buildbot controller process to kill it. |
|
Follow up for those not watching the buildbots. Before this PR: |
|
Likewise for the other Windows 10 buildbot (bolen-windows10), before: |
Yes, this is expected due to the current goings on with bpo-11105. The tests are aborting, if you scroll back to their initial test. The whole point of this PR is to see these failures from the buildbots instead of being masked by timeout exceptions from the builder process. Note that I cannot reproduce any aborts locally when using a release build, however I am running Windows build 19043 (21H1) vs 17763 on the CI. The OS version differences have been known to influence test results. |
|
There is one problem with the approach in the PR, test__xxsubinterpreters (specifically the is_running() function) cannot be run sequentially, that is, in the same process as the test runner. It fails when more than one thread state is active. It would be possible to skip this particular test case when not run in sub-process mode (-jN) with some additional plumbing. If that approach is deemed worthy, I'll update the PR to include that as well. I really would like to see the failures on the Windows buildbots show prominently again, instead of being masked behind builder timeout exceptions. |
This prevents inadvertent interactions with tests expecting a single threaded environment. Displaying load is really only hel 8000 pful for buildbots running in multiprocess mode anyway.
|
I've addressed the conflicts with test__xxsubinterpreters in the latest commit. In short, only use load for multiprocess test runs. |
|
This PR is stale because it has been open for 30 days with no activity. |
|
This would be less fragile and more useful in general if support were added to |
|
Sorry @jkloth and @zooba, I had trouble checking out the |
|
Sorry, @jkloth and @zooba, I could not cleanly backport this to |
|
Sorry @jkloth and @zooba, I had trouble checking out the |
|
GH-32048 is a backport of this pull request to the 3.10 branch. |
…indows (pythonGH-26578) Replace the child process `typeperf.exe` with a daemon thread that reads the performance counters directly. This prevents the issues that arise from inherited handles in grandchild processes (see issue37531 for discussion). We only use the load tracker when running tests in multiprocess mode. This prevents inadvertent interactions with tests expecting a single threaded environment. Displaying load is really only helpful for buildbots running in multiprocess mode anyway. (cherry picked from commit 19058b9) Co-authored-by: Jeremy Kloth <jeremy.kloth@gmail.com>
|
GH-32050 is a backport of this pull request to the 3.9 branch. |
…ndows (pythonGH-26578) Replace the child process `typeperf.exe` with a daemon thread that reads the performance counters directly. This prevents the issues that arise from inherited handles in grandchild processes (see issue37531 for discussion). We only use the load tracker when running tests in multiprocess mode. This prevents inadvertent interactions with tests expecting a single threaded environment. Displaying load is really only helpful for buildbots running in multiprocess mode anyway.. (cherry picked from commit 19058b9) Co-authored-by: Jeremy Kloth <jeremy.kloth@gmail.com>
…H-26578) Replace the child process `typeperf.exe` with a daemon thread that reads the performance counters directly. This prevents the issues that arise from inherited handles in grandchild processes (see issue37531 for discussion). We only use the load tracker when running tests in multiprocess mode. This prevents inadvertent interactions with tests expecting a single threaded environment. Displaying load is really only helpful for buildbots running in multiprocess mode anyway. Co-authored-by: Jeremy Kloth <jeremy.kloth@gmail.com>
…H-26578) Replace the child process `typeperf.exe` with a daemon thread that reads the performance counters directly. This prevents the issues that arise from inherited handles in grandchild processes (see issue37531 for discussion). We only use the load tracker when running tests in multiprocess mode. This prevents inadvertent interactions with tests expecting a single threaded environment. Displaying load is really only helpful for buildbots running in multiprocess mode anyway.. Co-authored-by: Jeremy Kloth <jeremy.kloth@gmail.com>
…ythonGH-26578) Replace the child process `typeperf.exe` with a daemon thread that reads the performance counters directly. This prevents the issues that arise from inherited handles in grandchild processes (see issue37531 for discussion). We only use the load tracker when running tests in multiprocess mode. This prevents inadvertent interactions with tests expecting a single threaded environment. Displaying load is really only helpful for buildbots running in multiprocess mode anyway.. Co-authored-by: Jeremy Kloth <jeremy.kloth@gmail.com>
Replace the child process
typeperf.exewith a daemon threadthat reads the performance counters directly. This prevents
the issues that arise from inherited handles in grandchild processes
(see https://bugs.python.org/issue37531#msg350191).
https://bugs.python.org/issue44336