Review request for 7034570

[Alan Bateman]

Michael McMahon wrote:
> This issue occurs on some versions of Windows since we switched compiler
> to VC 2010. The new C runtime library requires the "SystemRoot" 
> environment
> variable to be set, when launching sub-processes via Runtime.exec() or 
> ProcessBuilder.
>
> The problem occurs in instances where the environment is specified by 
> the caller,
> rather than simply inheriting (or inheriting and adding to) the 
> environment of the parent
> process.
>
> The change is simple enough. We just check for the presence of 
> "SystemRoot" in the
> child process's environment, and set it if it's not there.  No change 
> will be visible
> to the parent process. But, to avoid ambiguity, we'd like to make the 
> change explicit
> with a small spec. change.
>
> http://cr.openjdk.java.net/~michaelm/7034570/webrev.1/
Thanks for taking this one. Although we might be seeing this issue more 
frequently since moving to the newer C runtime, I'm pretty sure that not 
having SystemRoot in the environment has always lead to an altered state 
of consciousness. At one point I recall that Windows Sockets could be 
initialized but would fail creating sockets when this variable wasn't 
set, something to do with how layered service providers were located via 
the registry if I recall.

Anyway, it's good to get this fixed. The update to the javadoc mostly 
looks good to me. One suggestion for ProcessBuilder.start is that 
"beyond those in the specified environment" doesn't quite work as the 
environment isn't specified to the method. How about "beyond those in 
the process builder's {@link #environment()}". The update to 
Runtime.exec looks good to me.

In toEnvironmentBlock does the getenv("SystemRoot") need to be done in a 
privileged block (I'm just thinking of the case where you have 
permissions to exec the process but not read the variable). Also do you 
need to handle the case that it is null?

It would be good to add the bugID to the list at the top of the test.

-Alan.