8000 GH-130727: test_wmi_query_error test is flaky by chris-eibl · Pull Request #134313 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

GH-130727: test_wmi_query_error test is flaky #134313

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 20, 2025
Merged

GH-130727: test_wmi_query_error test is flaky #134313

merged 4 commits into from
May 20, 2025

Conversation

chris-eibl
Copy link
Member
@chris-eibl chris-eibl commented May 20, 2025 •
edited
Loading

I can reproduce the the "invalid handle" errors which @vstinner has also seen #130727 (comment) under high load (using "Stress CPU" of CPU-Z) running

python -m test test_wmi -m test.test_wmi.WmiTests.test_wmi_query_repeated -F

The reason is, that the main thread calls CloseHandle on the still running thread in case of a WAIT_TIMEOUT, and then the thread dies with

Exception thrown at 0x00007FFE1A35144A (ntdll.dll) in python_d.exe: 0xC0000008: An invalid handle was specified.

and call stack

 	ntdll.dll!00007ffe1a35144a()	Unknown
 	KernelBase.dll!00007ffe17cd0905()	Unknown
>	_wmi_d.pyd!_query_thread(void * param) Line 204	C++

where

    CloseHandle(data->writePipe); //Line 204
    return (DWORD)hr;

when I debug it in Visual Studio.

This has already been anticipated by @runn in #125315 (comment).

Another manifestation of this race results in

======================================================================
FAIL: test_wmi_query_repeated (test.test_wmi.WmiTests.test_wmi_query_repeated)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "R:\buildarea\3.14.ambv-bb-win11.bigmem\build\Lib\test\test_wmi.py", line 41, in test_wmi_query_repeated
    self.test_wmi_query_os_version()
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "R:\buildarea\3.14.ambv-bb-win11.bigmem\build\Lib\test\test_wmi.py", line 30, in test_wmi_query_os_version
    self.assertEqual(1, len(r))
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^
AssertionError: 1 != 2

which sometimes occur on the build bots, e.g. https://buildbot.python.org/#/builders/1799/builds/20.

Copying the _query_data struct in the wmi thread fixes both issues.

@chris-eibl chris-eibl requested a review from a team as a code owner May 20, 2025 05:39
@bedevere-app bedevere-app bot mentioned this pull request May 20, 2025
Closed
@chris-eibl chris-eibl requested a review from zooba May 20, 2025 05:40
@chris-eibl
Copy link
Member Author

@zooba: I am unsure whether to associate this PR with #130727 or #125315, I went with the former. In that case, I think it doesn't need a news entry, since it is just fixing a flaky test, but I'd happily blurb it if it deems appropriate.

@zooba
Copy link
Member
zooba commented May 20, 2025

@chris-eibl Yeah, give it a NEWS entry, since it's a change to the runtime rather than the tests, and so users might see it out in the wild and wonder about the changed behaviour.

Before merging, I'd want to confirm that there's no intentional modification of that struct that is supposed to be picked up by the other thread. I suspect not, but I haven't read the code to double-check.

@zooba zooba merged commit e4fbfb1 into python:main May 20, 2025
39 checks passed
@zooba zooba added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels May 20, 2025
@miss-islington-app
Copy link

Thanks @chris-eibl for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @chris-eibl for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 20, 2025
@chris-eibl @miss-islington
pythonGH-130727: Avoid race condition in _wmimodule by copying shared…
24019bb 
… data (pythonGH-134313)

(cherry picked from commit e4fbfb1)

Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 20, 2025
@chris-eibl @miss-islington
pythonGH-130727: Avoid race condition in _wmimodule by copying shared…
ecab5f3 
… data (pythonGH-134313)

(cherry picked from commit e4fbfb1)

Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com>
@bedevere-app
Copy link
bedevere-app bot commented May 20, 2025

GH-134396 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label May 20, 2025
@bedevere-app
Copy link
bedevere-app bot commented May 20, 2025

GH-134397 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label May 20, 2025
@chris-eibl chris-eibl deleted the fix_wmi_race branch May 20, 2025 21:23
zooba pushed a commit that referenced this pull request May 20, 2025
@miss-islington @chris-eibl
GH-130727: Avoid race condition in _wmimodule by copying shared data (G…
35eb3b8 
…H-134313)

(cherry picked from commit e4fbfb1)

Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com>
zooba pushed a commit that referenced this pull request May 20, 2025
@miss-islington @chris-eibl
GH-130727: Avoid race condition in _wmimodule by copying shared data (G…
81c348b 
…H-134313)

(cherry picked from commit e4fbfb1)

Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com>
lkollar pushed a commit to lkollar/cpython that referenced this pull request May 26, 2025
@chris-eibl @lkollar
pythonGH-130727: Avoid race condition in _wmimodule by copying shared…
9ae095a 
… data (pythonGH-134313)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zooba zooba zooba left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chris-eibl @zooba
0