Here is new version of the fix:
http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-7147084/webrev.02/
Changes (in accordance with Alan's recommendations):
-- Magic constant -1 was changed to JAVA_INVALID_HANDLE_VALUE with
explanation why the macro
INVALID_HANDLE_VALUE is not in use.
-- The comments were added for each usage of the
JAVA_INVALID_HANDLE_VALUE macro.
-- 10 seconds time limit was rewritten to file signals.
Regards,
-uta
On 7/29/2013 2:32 AM, Alan Bateman wrote:
On 11/07/2013 02:03, Alexey Utkin wrote:
:
Bug description:
https://jbs.oracle.com/bugs/browse/JDK-7147084
http://bugs.sun.com/view_bug.do?bug_id=7147084
Here is the suggested fix:
http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-7147084/webrev.01/
Summary for v1 changes:
-- The set of handles that need to restore the inherit flag was extended
by child process IOE handles. That was done to avoid "greedy sibling"
problem for the file handles. The file handles are closed outside the
synchronized block.
-- "Greedy sibling" problem is covered by the
[test/java/lang/ProcessBuilder/SiblingIOEHandle.java] test.
-- The fact that the set of current process standard IOE handles and the
set of child process IOE handles can intersect was taken into account.
-- The [test/java/lang/ProcessBuilder/InheritIOEHandle.java] was changed
in accordance with Martin's concern.
I've done a first pass over this. This is a long standing issue so a
big thank you for taking this on. It's unfortunate that this requires
serializing the exec but based on the various Microsoft articles that
you cited when researching this one then it seems that this is what we
have to do.
Overall I don't see any issues. It's nice to have the create function
broken up into smaller functions. I do think the new code needs
several comments, particularly initHolder and the constants.
One concern with the tests is that 10 seconds might not be sufficient
on a slow/busy machine or a Windows machine that is being choked by AV
software. Could this be changed so that it just getting timed by the
test harness if the child does not terminate? I'm also wondering about
waitAbit and whether this is useful.
-Alan.
