Alan Bateman wrote:
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.
Ok.
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?
The permission check happens at a higher level. We're dealing directly
with the data at this level.
So, we don't need a privileged block.
Yes, the null case needs to be distinguished.
I'll send another webrev soon.
Thanks,
Michael.