Hello Alan, Martin, everyone!

Some update on the issue.
When trying to integrate my test into Basic.java, I found that it already contains exactly the same.
I have just overlooked it before.

Additional investigation showed that inheritIO() doesn't always fail on Windows.
If the scenario is like following:
- java application creates a child java process (no IO inheritance, redirecting IO through pipes by default), - child process creates a grandchild which inherits the child's stdin/out/err,
everything works as expected.

However, if the java app (started from a console) creates an immediate child that inherits its parent stdin/out/err, it fails.

That's why this bug hasn't been caught by Basic.java before - because it uses the first testing scenario. And that's why I had to introduce a new shell script test - because the problem couldn't be reproduced otherwise.

Would you please review the updated webrev?
http://cr.openjdk.java.net/~igerasim/8023130/1/webrev/ <http://cr.openjdk.java.net/%7Eigerasim/8023130/1/webrev/>

The proposed fix remained the same.
I only created a new shell script that reuses Basic.java functionality.
I also made a minor change to Basic.java to include testing of ProcessBuilder#redirectXXX(Redirect.INHERIT) method calls.

I manually tested the fixed jdk to check that we haven't reintroduced 4244515 bug with this fix. As Martin said, unfortunately there is no automated test for windowed apps behavior.

Sincerely yours,
Ivan Gerasimov


On 26.08.2013 3:31, Martin Buchholz wrote:
Historical notes:

I maintained this code for a while, but I've never been a "Windows" guy, and I rarely sat at an actual Windows machine console. I don't know of any test infrastructure for windowed apps.


On Fri, Aug 23, 2013 at 9:11 AM, Ivan Gerasimov <ivan.gerasi...@oracle.com <mailto:ivan.gerasi...@oracle.com>> wrote:

    Thank you Alan!


    On 23.08.2013 14:28, Alan Bateman wrote:

        On 23/08/2013 04:07, Ivan Gerasimov wrote:

            Hello everybody!

            The method ProcessBuilder#inheritIO() is reported to not
            have any effect on Windows platform.
            The same story is with
            redirectOutput/Input/Error(Redirect.INHERIT) methods.
            As the result, standard in/out/err aren't inherited.

            It turn out that the culprit is the CREATE_NO_WINDOW flag
            passed to CreateProcessW().
            MS doc says about this flag: "The process is a console
            application that is being run without a console window."

            CREATE_NO_WINDOW flag was added with the fix for
            http://bugs.sun.com/view_bug.do?bug_id=4244515 to suppress
            a console window on a newly created process, when it is
            created from a program launched with javaw.exe.
            Thus, I left this flag only for cases when the program
            doesn't have a console associated with it.

            Would you please help review a fix for this problem?

            BUGURL: http://bugs.sun.com/view_bug.do?bug_id=8023130
            WEBREV:
            http://cr.openjdk.java.net/~igerasim/8023130/0/webrev/
            <http://cr.openjdk.java.net/%7Eigerasim/8023130/0/webrev/>

        Good sleuthing!

        Just so I understand, if we have a console (DOS command window
        for example) then will dropping CREATE_NO_WINDOW result in a
        new window or not?

    No new console for a console application without CREATE_NO_WINDOW
    flag.
    I used a sample program to confirm that:
    ---------------
    class InheritIO {
        public static void main(String[] args) throws Exception {
            int err = new
    
ProcessBuilder(args).inheritIO().redirectErrorStream(true).start().waitFor();
            System.err.println("Exit value: " + err);
        }
    }
    ---------------
    With the proposed fix running it as '> java InheritIO cmd /?'
    copies the output of the command to the existing console.
    No new console is created.


        One thing that it does highlight is that the coverage for
        inherit in ProcessBuilder/Basic.java was not sufficient as
        this should have been caught a long time ago. My preference
        would be to add to this test rather than introduce a new one
        (ProcessBuilder.java is Martin's original mother-of-all tests
        for ProcessBuilder).

    Yes, I agree. It would be better to integrate this test into
    Basic.java.

    Sincerely yours,
    Ivan

        -Alan.





Reply via email to