Commons Exec
  1. Commons Exec
  2. EXEC-33

PumpStreamHandler hangs if System.in is redirect to process input stream

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.0
    • Fix Version/s: 1.0.1
    • Labels:
      None
    • Environment:

      Windows Vista

      Description

      When process input is redirected using a PumpStreamHandler, e.g.

      PumpStreamHandler streamHanlder = new PumpStreamHandler(out, err, System.in);
      exec.setStreamHandler(streamHanlder);
      MockExecuteResultHandler handler = new MockExecuteResultHandler();
      exec.execute(cl, handler);

      the process hangs and never exit.

      1. EXEC33.patch
        5 kB
        Milos Kleint
      2. PumpStreamHandler.patch
        0.6 kB
        Marco Ferrante

        Activity

        Hide
        Siegfried Goeschl added a comment - - edited

        Confirmed so far ...

        +) the following test script hangs
        +) one worker thread stll hang in StreamPumper.java
        +) Having said this I have a working test case for redirecting a FileInputStream instead of System.in so I'm a bit pussled

         
            /**
             * Start a process and connect stdin, stdout and stderr. This 
             * test currenty hang ....
             */
            public void testExecuteWithStdin() throws Exception
            {
                CommandLine cl = new CommandLine(testScript);
                PumpStreamHandler pumpStreamHandler = new PumpStreamHandler( System.out, System.err, System.in );
                DefaultExecutor executor = new DefaultExecutor();
                executor.setStreamHandler( pumpStreamHandler );
                int exitValue = executor.execute(cl);
                assertFalse(exec.isFailure(exitValue));
            }
        
        Show
        Siegfried Goeschl added a comment - - edited Confirmed so far ... +) the following test script hangs +) one worker thread stll hang in StreamPumper.java +) Having said this I have a working test case for redirecting a FileInputStream instead of System.in so I'm a bit pussled /** * Start a process and connect stdin, stdout and stderr. This * test currenty hang .... */ public void testExecuteWithStdin() throws Exception { CommandLine cl = new CommandLine(testScript); PumpStreamHandler pumpStreamHandler = new PumpStreamHandler( System.out, System.err, System.in ); DefaultExecutor executor = new DefaultExecutor(); executor.setStreamHandler( pumpStreamHandler ); int exitValue = executor.execute(cl); assertFalse(exec.isFailure(exitValue)); }
        Hide
        Benjamin Bentmann added a comment -

        The problematic code can be reduced to:

        InputStream is = System.in;
        OutputStream os = System.out;
        
        final byte[] buf = new byte[4096];
        int length;
        while ((length = is.read(buf)) > 0) {
            os.write(buf, 0, length);
        }
        System.out.println("DONE");
        

        i.e. this hangs, too. It appears that's a basic Java issue, cf. Sun bugs #4514257, #4385444 and related. Basically System.in is an open but empty stream, so the call InputStream.read() will block infinitely. As the mentioned Sun issues point out, Thread.interrupt() doesn't help either once the blocking call to read() has been made.

        InputStream.available() seems the only non-blocking way to detect the empty state of the stream but unfortunately it doesn't detect whether the stream is closed.

        Maybe one needs a special StreamPumper (e.g. subclass) for the inputThread that pumps the streams via polling, i.e. available() + read() + Thread.sleep(), and gets terminated when interrupted from outside, i.e. by PumpStreamHandler.stop(), instead of stream closing. When the subprocess has exited, there is no need to pump further input into it, so terminating the inputThread in PumpStreamHandler.stop() without consuming all of the input shouldn't be a problem. Surely, beauty is something else.

        Show
        Benjamin Bentmann added a comment - The problematic code can be reduced to: InputStream is = System .in; OutputStream os = System .out; final byte [] buf = new byte [4096]; int length; while ((length = is.read(buf)) > 0) { os.write(buf, 0, length); } System .out.println( "DONE" ); i.e. this hangs, too. It appears that's a basic Java issue, cf. Sun bugs #4514257 , #4385444 and related. Basically System.in is an open but empty stream, so the call InputStream.read() will block infinitely. As the mentioned Sun issues point out, Thread.interrupt() doesn't help either once the blocking call to read() has been made. InputStream.available() seems the only non-blocking way to detect the empty state of the stream but unfortunately it doesn't detect whether the stream is closed. Maybe one needs a special StreamPumper (e.g. subclass) for the inputThread that pumps the streams via polling, i.e. available() + read() + Thread.sleep() , and gets terminated when interrupted from outside, i.e. by PumpStreamHandler.stop() , instead of stream closing. When the subprocess has exited, there is no need to pump further input into it, so terminating the inputThread in PumpStreamHandler.stop() without consuming all of the input shouldn't be a problem. Surely, beauty is something else.
        Hide
        Joerg Schaible added a comment - - edited

        As the mentioned Sun issues point out, Thread.interrupt() doesn't help either once the blocking call to read() has been made.

        This is actually not completely true. It depends on the implementation of the JVM and the underlaying platform. On Solaris read() will be interrupted and throwing an InterruptedIOException. So be prepared to deal with both situations.

        Show
        Joerg Schaible added a comment - - edited As the mentioned Sun issues point out, Thread.interrupt() doesn't help either once the blocking call to read() has been made. This is actually not completely true. It depends on the implementation of the JVM and the underlaying platform. On Solaris read() will be interrupted and throwing an InterruptedIOException. So be prepared to deal with both situations.
        Hide
        Benjamin Bentmann added a comment -

        It depends on the implementation of the JVM and the underlaying platform. On Solaris read() will be interrupted

        Correct, additionally depends on the JVM flags passed in (UseVMInterruptibleIO).

        My intention was merely to point out that we have no reliable/general/platform-independent way of interrupting read().

        Show
        Benjamin Bentmann added a comment - It depends on the implementation of the JVM and the underlaying platform. On Solaris read() will be interrupted Correct, additionally depends on the JVM flags passed in (UseVMInterruptibleIO). My intention was merely to point out that we have no reliable/general/platform-independent way of interrupting read() .
        Hide
        Siegfried Goeschl added a comment -

        I think the checking System.in might solve the problem since it would be a bug to close stdin anyway - so having a smarter StreamPumper might be worth the effort. Many thanks for Benjamin and Joerg for helping me on this issue .....

        Show
        Siegfried Goeschl added a comment - I think the checking System.in might solve the problem since it would be a bug to close stdin anyway - so having a smarter StreamPumper might be worth the effort. Many thanks for Benjamin and Joerg for helping me on this issue .....
        Hide
        Marco Ferrante added a comment -

        If I understand your suggestion correctly, this patch should be enough to work around the problem; at least, it passes the test case.

        Show
        Marco Ferrante added a comment - If I understand your suggestion correctly, this patch should be enough to work around the problem; at least, it passes the test case.
        Hide
        Siegfried Goeschl added a comment -

        Well, you are dodging the bullet - the question is can you do something meaningful with the redirected System.in in your application?

        Show
        Siegfried Goeschl added a comment - Well, you are dodging the bullet - the question is can you do something meaningful with the redirected System.in in your application?
        Hide
        Siegfried Goeschl added a comment -

        Fixing this correctly on all platforms does not look easy so I will look at it after the first release.

        Show
        Siegfried Goeschl added a comment - Fixing this correctly on all platforms does not look easy so I will look at it after the first release.
        Hide
        Siegfried Goeschl added a comment -

        Throwing an IllegalArgumentException when System.in is used to notify the user ....

        Show
        Siegfried Goeschl added a comment - Throwing an IllegalArgumentException when System.in is used to notify the user ....
        Hide
        Milos Kleint added a comment -

        attached is a suggested patch that was tested with a modified exec-maven-plugin (modified to use commons-exec instead of plexus-utils for execution)

        Show
        Milos Kleint added a comment - attached is a suggested patch that was tested with a modified exec-maven-plugin (modified to use commons-exec instead of plexus-utils for execution)
        Hide
        Benjamin Bentmann added a comment -

        Milos, shouldn't the inner while loop of InputStreamPumper.run()} also check the {{stop flag to make sure processing stops when requested and not when the input is exhausted?

        Show
        Benjamin Bentmann added a comment - Milos, shouldn't the inner while loop of InputStreamPumper.run()} also check the {{stop flag to make sure processing stops when requested and not when the input is exhausted?
        Hide
        Milos Kleint added a comment -

        Benjamin, that would be an improvement for sure. Typically you wouldn't type as fast as 10 characters per second, but you can surely copy&paste a large amount of text and then it would be more than useful to the the inner loop to stop as well.

        Show
        Milos Kleint added a comment - Benjamin, that would be an improvement for sure. Typically you wouldn't type as fast as 10 characters per second, but you can surely copy&paste a large amount of text and then it would be more than useful to the the inner loop to stop as well.
        Hide
        Siegfried Goeschl added a comment -

        Milos, I'm completely busy the next few days so it looks like next weekend for the patch ....

        Show
        Siegfried Goeschl added a comment - Milos, I'm completely busy the next few days so it looks like next weekend for the patch ....
        Hide
        Siegfried Goeschl added a comment -

        Milos, I applied the patch - can you make a quick retest so I can plan a release ...

        Show
        Siegfried Goeschl added a comment - Milos, I applied the patch - can you make a quick retest so I can plan a release ...
        Hide
        Milos Kleint added a comment -

        yes, it works.
        I have updated svn, rebuilt commons-exec. Used the snapshot in exec-mojo-plugin (wit simplified patch from http://jira.codehaus.org/browse/MEXEC-25.

        used that exec plugin in netbeans for an app that processes output. Seems to work as expected.

        Show
        Milos Kleint added a comment - yes, it works. I have updated svn, rebuilt commons-exec. Used the snapshot in exec-mojo-plugin (wit simplified patch from http://jira.codehaus.org/browse/MEXEC-25 . used that exec plugin in netbeans for an app that processes output. Seems to work as expected.
        Hide
        Milos Kleint added a comment -

        is the new release still in the pipeline?

        Show
        Milos Kleint added a comment - is the new release still in the pipeline?
        Hide
        Siegfried Goeschl added a comment -

        Yes - I was sucked up in a customer project for the last two months but workload becomes normal. So I will cut an RC next week

        Show
        Siegfried Goeschl added a comment - Yes - I was sucked up in a customer project for the last two months but workload becomes normal. So I will cut an RC next week
        Hide
        Milos Kleint added a comment -

        ping?

        I need the commons-exec released to have exec-maven-plugin released to be included in the next netbeans release. Quite a long chain, that's why I keep on pushing..

        Show
        Milos Kleint added a comment - ping? I need the commons-exec released to have exec-maven-plugin released to be included in the next netbeans release. Quite a long chain, that's why I keep on pushing..
        Hide
        Siegfried Goeschl added a comment -

        I'm starting to cut an RC for commons-exec-1.0.1 ...

        Show
        Siegfried Goeschl added a comment - I'm starting to cut an RC for commons-exec-1.0.1 ...
        Hide
        Milos Kleint added a comment -

        any updates?

        The bug is still open, does it mean the patch hasn't been applied yet?

        Show
        Milos Kleint added a comment - any updates? The bug is still open, does it mean the patch hasn't been applied yet?
        Hide
        Arnaud HERITIER added a comment -

        Hi, do you know when you'll fix this issue and when you'l release the 1.0.1 ?
        It's really annoying to not being able to fix the issue we have in the Exec Mojo.
        If it's too long, we'll have to see if we can use another library or if we fork yours.

        Show
        Arnaud HERITIER added a comment - Hi, do you know when you'll fix this issue and when you'l release the 1.0.1 ? It's really annoying to not being able to fix the issue we have in the Exec Mojo. If it's too long, we'll have to see if we can use another library or if we fork yours.
        Hide
        Siegfried Goeschl added a comment -

        The patch is long applied but my release candidate got stuck due to M2 and SVN problems many moons ago ... I give it another go now

        Show
        Siegfried Goeschl added a comment - The patch is long applied but my release candidate got stuck due to M2 and SVN problems many moons ago ... I give it another go now
        Hide
        Arnaud HERITIER added a comment -

        ok, let us know if we can help you.

        Show
        Arnaud HERITIER added a comment - ok, let us know if we can help you.
        Hide
        Henri Biestro added a comment -

        Btw, the same pb occured on Mac OSX with unit tests hanging, even with the fix.

        Committed a modification:
        URL: http://svn.apache.org/viewvc?rev=818863&view=rev
        Log:
        On a Mac, the unit tests never finish. Culprit is InputStreamPumper which sets its stop member in the run method; however, run might really be executed after the stopProcessing method is called if the process thread completes before the InputStreamPumper starts.

        Modified:
        commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/InputStreamPumper.java

        Show
        Henri Biestro added a comment - Btw, the same pb occured on Mac OSX with unit tests hanging, even with the fix. Committed a modification: URL: http://svn.apache.org/viewvc?rev=818863&view=rev Log: On a Mac, the unit tests never finish. Culprit is InputStreamPumper which sets its stop member in the run method; however, run might really be executed after the stopProcessing method is called if the process thread completes before the InputStreamPumper starts. Modified: commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/InputStreamPumper.java
        Hide
        Siegfried Goeschl added a comment -

        Added a line in changes.xml to document the modification

        Show
        Siegfried Goeschl added a comment - Added a line in changes.xml to document the modification
        Hide
        Siegfried Goeschl added a comment -

        Hi Arnaud and Milos,

        I'm simply unable to bring the RC out of the door - I have now SSH issues after solving my SVN problems. Either you have some more patience while I'm sorting out the infrastructure or you make a private fork

        Sorry for the inconvinience

        Show
        Siegfried Goeschl added a comment - Hi Arnaud and Milos, I'm simply unable to bring the RC out of the door - I have now SSH issues after solving my SVN problems. Either you have some more patience while I'm sorting out the infrastructure or you make a private fork Sorry for the inconvinience
        Hide
        Arnaud HERITIER added a comment -

        I prefer to wait some days/weeks before to create a private fork. Thx for your help. I hope your ssh issue will be fixed soon.

        Show
        Arnaud HERITIER added a comment - I prefer to wait some days/weeks before to create a private fork. Thx for your help. I hope your ssh issue will be fixed soon.
        Hide
        Brett Porter added a comment -

        Siegfried - did you sort them out? I think Phil's comment on commons-dev will help - if there's anything you need then please let me know.

        Show
        Brett Porter added a comment - Siegfried - did you sort them out? I think Phil's comment on commons-dev will help - if there's anything you need then please let me know.
        Hide
        Siegfried Goeschl added a comment -

        One more task for late night action hero - upgrade to Maven 2.2.1 to get the release candidate going ...

        Show
        Siegfried Goeschl added a comment - One more task for late night action hero - upgrade to Maven 2.2.1 to get the release candidate going ...

          People

          • Assignee:
            Siegfried Goeschl
            Reporter:
            Marco Ferrante
          • Votes:
            1 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development