bpo-30121: Fix debug assert in subprocess on Windows #1224
Conversation
|
@segevfiner, thanks for your PR! By analyzing the history of the files in this pull request, we identified @gpshead, @tim-one and @rosslagerwall to be potential reviewers. |
|
ping @Haypo since you marked yourself for review some time ago but might have forgotten about this 😉 |
This is caused by closing HANDLEs using os.close which is for CRT file descriptors and not for HANDLEs.
|
@Haypo Plenty of tests already trigger this. And any simple test that just tries to run a non-existent executable with io redirection will also trigger this. But libregrtest does I did encounter one test in I don't know how to write a test that checks specifically that there isn't a debug assertion under |
72ed71a to
6dd5a06
Compare
|
Sorry, it took me a while to understand the fixed bug, but now I understood the 3 changes and they all LGTM. I had to play with subprocess to understand why we need all these changes. |
|
I wrote an unit test for this change: PR #3133. |
…3173) * bpo-30121: Fix debug assert in subprocess on Windows (#1224) * bpo-30121: Fix debug assert in subprocess on Windows This is caused by closing HANDLEs using os.close which is for CRT file descriptors and not for HANDLEs. * bpo-30121: Suppress debug assertion in test_subprocess when ran directly (cherry picked from commit 4d38517) * Add test_subprocess.test_nonexisting_with_pipes() (#3133) bpo-30121: Test the Popen failure when Popen was created with pipes. Create also NONEXISTING_CMD variable in test_subprocess.py. (cherry picked from commit 9a83f65)
This is caused by closing HANDLEs using os.close which is for CRT file
descriptors and not for HANDLEs.
http://bugs.python.org/issue30121