[Python-Dev] Issue 13524: subprocess on Windows

[Tim Golden]

http://bugs.python.org/issue13524

Someone raised issue13524 yesterday to illustrate that a
subprocess will crash immediately if an environment block is
passed which does not contain a valid SystemRoot environment
variable.

Note that the calling (Python) process is unaffected; this
isn't - strictly - a Python crash. The issue is essentially
a Windows one where a fairly unusual cornercase -- passing
an empty environment -- has unforseen effects.

The smallest reproducible example is this:

import os, sys
import subprocess
subprocess.Popen(
     [sys.executable],
     env={}
)

and it can be prevented like this:

import os, sys
import subprocess
subprocess.Popen(
     [sys.executable],
     env={"SystemRoot" : os.environ['SystemRoot']}
)

There's a blog post here which gives a worked example:

 
http://jpassing.com/2009/12/28/the-hidden-danger-of-forgetting-to-specify-systemroot-in-a-custom-environment-block/

but as the author points out, nowhere on MSDN is there a warning
that SystemRoot is mandatory. (And, in effect, it's not as it
would just be possible to write code which had no need of it).

So... what's our take on this? As I see it we could:

1) Do nothing: it's the caller's responsibility to understand the
    complications of the chosen Operating System.

2) Add a doc warning (ironically, considering the recent to-and-fro
    on doc warnings in this very module).

3) Add a check into the subprocess.Popen code which would raise some
    exception if the environment block is empty (or doesn't contain
    SystemRoot) on the grounds that this probably wasn't what the user
    thought they were doing.

4) Automatically add an entry for SystemRoot to the env block if it's
    not present already.


It's tempting to opt for (1) and if we were exposing an API called
CreateProcess which mimicked the underlying Windows API I would be
inclined to go that way. But we're abstracting a little bit away
from that and I think that that layer of abstraction carries its
own responsibilities.

Option (3) seems to give the best balance. It *is* a cornercase, but at
the same time it's easy to misunderstand that the env block you're
passing in *replaces* rather than *augments* that of the current
process.

Thoughts?

TJG