subprocess.run(..., env={}) broken on Windows

[mandolaerik]

Bug report

When passing an empty dictionary as env to subprocess.run (or Popen), I get the following error:

Python 3.11.3 (tags/v3.11.3:f3909b8, Apr  4 2023, 23:49:59) [MSC v.1934 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import subprocess
>>> subprocess.run('c:\\msys64\\usr\\bin\\echo.exe', env={})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\jhbaarnh\AppData\Local\Programs\Python\Python311\Lib\subprocess.py", line 548, in run
    with Popen(*popenargs, **kwargs) as process:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\jhbaarnh\AppData\Local\Programs\Python\Python311\Lib\subprocess.py", line 1024, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "C:\Users\jhbaarnh\AppData\Local\Programs\Python\Python311\Lib\subprocess.py", line 1509, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
OSError: [WinError 87] The parameter is incorrect
>>>

Environment

• Reproduced on the following CPython versions, all native windows builds:
  • vanilla python 3.11.3
  • vanilla Python 3.9.6
  • Intel Python 3.9.16
• I can not reproduce the problem on any of the following:
  • python 3.7.2, native Windows build and MSYS build (did not try newer MSYS)
  • any linux

Linked PRs

• gh-105436： the environment block should end with two zero-valued wchar_t values #105495
• [3.12] gh-105436: The environment block should end with two null wchar_t values (GH-105495) #105700
• [3.11] gh-105436: The environment block should end with two null wchar_t values (GH-105495) #105701
• gh-105436: Ignore unrelated errors when checking empty env #105742
• [3.12] gh-105436: Ignore unrelated errors when checking empty env (GH-105742) #105756
• [3.11] gh-105436: Ignore unrelated errors when checking empty env (GH-105742) #105757

[sku2000]

The error occurs in the _winapi_CreateProcess_impl function of cpython/Modules/_winapi.c.
When the CreateProcessW function is called, the wenvironment argument ends with a single 0 byte.
In the CreateProcessW document, the ANSI environment block ends with two zero bytes:
Note that an ANSI environment block is terminated by two zero bytes

[eryksun]

@sku2000

Yes, CreateProcessW() fails with ERROR_INVALID_PARAMETER (87) if the environment block doesn't end with two nulls (i.e. two zero-valued wchar_t values, which is four zero-valued bytes), one for the last variable and one for the block. Thus even an empty block has to have an empty variable assignment. Currently if the env mapping is empty, we create a block that only has a single null. Probably the simplest fix would be to special case the result of getenvironment() in "Modules/_winapi.c" when envsize is 0.

cpython/Modules/_winapi.c

Lines 798 to 806 in 7279fb6

	envsize = PyList_GET_SIZE(keys);
	if (PyList_GET_SIZE(values) != envsize) {
	PyErr_SetString(PyExc_RuntimeError,
	"environment changed size during iteration");
	goto error;
	}
	totalsize = 1; /* trailing null character */
	for (i = 0; i < envsize; i++) {

[zooba]

Thanks for the report and to @sku2000 for the fix!

[zooba]

The test fails in situations where processes can't launch without a valid environment (fairly common on Windows).

We should replace the test with one that checks that OSError(winerror=87) is not raised. Any other error can be ignored.

[sku2000]

If we don't care about the exit status of the chid process, we can modify it like this:

@unittest.skipUnless(mswindows, "Maybe test trigger a leak on Ubuntu")
def test_run_with_an_empty_env(self):
    # gh-105436: fix subprocess.run(..., env={}) broken on Windows
    args = [sys.executable, "-c", 'import sys; sys.exit(57)']
    subprocess.run(args, env={})

Don't have to ignore other exceptions.

[eryksun]

I'd prefer for the new test to be placed immediately after the existing case test_empty_env(), which is skipped on Windows, and also to keep the name consistent, such as test_empty_env_windows(). At a minimum it should test that calling CreateProcessW() succeeds with an empty environment block. If the child succeeds, however, then it can also verify the child's os.environ, as is done in test_empty_env(). For example:

    @unittest.skipUnless(mswindows, "relaxed test for Windows")
    def test_empty_env_windows(self):
        # gh-105436: fix subprocess.run(..., env={}) on Windows
        args = [sys.executable, '-X', 'utf8', '-c',
                'import os; print(list(os.environ))']
        p = subprocess.run(args, env={}, capture_output=True)
        # Executing Python may depend on SystemRoot and/or PATH. If the child
        # succeeds, which should be the case for 3.12+ on x86/x64, then
        # verify that it was started with an empty environment.
        if p.returncode == 0:
            self.assertEqual(eval(p.stdout), [])

[zooba]

Further enhancing the test is fine, I'm just unblocking buildbots for now.