the following ANT code always hangs on Windows XP: <target name="start"> <echo message="asadmin start-domain" /> <exec dir="." executable="${asadmin}"> <arg value="start-domain"/> <arg value="${glassfish.domain}"/> </exec> <echo message="asadmin start-domain done" /> </target> My workaround: 1. PumpStreamHandler.stop() add a time out to the join() methods: try { outputThread.join(1000); } catch (InterruptedException e) { // ignore } try { errorThread.join(1000); } catch (InterruptedException e) { // ignore } 2. Execute.java a) fix the bogus method public static void closeStreams(Process process) { process.destroy(); // this is sufficient to cleanup all resources. /** WTF: never close streams maintained elsewhere! FileUtils.close(process.getInputStream()); FileUtils.close(process.getOutputStream()); FileUtils.close(process.getErrorStream()); */ } b) reorder the invokations public int execute() throws IOException { ... // streamHandler.stop(); closeStreams(process); // FIRST DESTROY THE PROCESS streamHandler.stop(); // THEN STOP THREADS ... works for me, maybe for you too.
*** This bug has been marked as a duplicate of bug 5003 ***
if there really is an issue caused by closing the streams than bug 46805 is no duplicate of bug 5003 Stefan, is this "just" a cosmetic issue like "you are not doing it correctly" or does it cause real problems?
It is a real an unresolved problem in ANT 1.7.1. Reproduced using Windows XP or Windows Vista(32/64). (You can easily find posts regarding this issue, just search "ant asadmin hang") So I fixed it locally and submitted my experience here.
I don't doubt that there is a bug in Ant 1.7.1, my question is whether Ant's trunk still exhibits the bug or whether not closing the streams is necessary to fix it.
This is definitely still a problem and the nightly build hasn't passed on Windows in months so there are no easily available downloads to test of trunk. Bug #5003 does seem similar but the closing of the streams is the issue here not the child processes leaving them open.
The real bug is to close streams which you never opened! foo = new XyzStream() --> foo.close(); foo = something.getStream() --> // nada Well, one could avoid problems by giving out proxied streams which ignore the close(), but obviously this is not the case. Why don't you simply add the suggested fix? Or fix it yourself?
(In reply to comment #5) > This is definitely still a problem and the nightly build hasn't passed on > Windows in months so there are no easily available downloads to test of trunk. Ant's nightly builds are platform independent, so you could try a different one.
the code in Ant's trunk is different enough from Ant 1.7.1 that I'd really like to have confirmation that not closing the streams is really fixing anything that has been broken before just to avoid adding yet another variable. The "don't close what you haven't created" argument doesn't really convince me. There are situations where you can't create a stream but are the only one who knows when to close it - in this situation you are the only one who can close the stream and should better do it. The Java class library has many such cases, see ZipFile.getInputStream or URLConnection.getInputStream. In some cases there is a hint of documentation of what is expected from the API user (ResultSet.get*Stream) but in most cases - including the Process.get*Stream methods - there is none. There simply doesn't seem to be a simple testcase that would allow us to test the current code base against anything that still exhibits problems and if you can't test trunk, we'll have to wait until the first release candidate for 1.8.0 is out.
Maybe I'm looking in the wrong place for a build, but http://ant.apache.org/nightlies.html has no "last successful" builds on Linux, Windows or Mac. I'm testing a build that runs on all three of these platforms, and only Windows has the problem described in this bug report. I'm currently resorting to coding my own ant task to handle asadmin execution properly. I'm happy to test another build of Ant though, I just need to know where to get it from (unless you mean build my own from SCM?)
I just checked out the latest trunk and built it locally and it still exhibits this bug.
Cory, have you ever tried 1.8.0 or 1.8.1?
The file src/tests/antunit/taskdefs/exec/exec-test.xml in Ant's svn contains an AntUnit test that demonstrates bug 5003 - it forks a process which in turn frokes a child process and exits after writing "finished". Ant has already incorporated the joint timeout and some other changes that ensure Ant doesn't wait for the forked child process (which sleeps for 20 seconds). The test is called testDoesntWaitForChildren If I apply the third part of Stefan's suggested patch we don't see the "finished" output anymore, that is we lose output. So far I hadn't seen that Stefan's second part called process.destroy - this seems to be not necessary since either process.waitFor has succeeded or process.destroy has been called in Execute#waitFor's exception handler. If I remove the closeStreams call completely I do get the "finished" line - or at least I seem to be getting it in repeated tests, there could still be a race condition. I'll run the full testsuite on Windows and Linux to see any ill effects show up.
Created attachment 25591 [details] Don't close process' streams I'm adding the trivial patch just for completeness. I can't confirm that it works for the bug we are talking about, but unfortunately it seems to cause problems at least on Linux - I'll have to investigate this further once I come to a physical rather than virtual test system.
> If I apply the third part of Stefan's suggested patch we don't see the > "finished" output anymore, that is we lose output. Losing the last output line might be a different bug, at least it often happens if ant is invoked via maven that the last output line gets suppressed. My workaround is to end every ant task with two identical echo messages. (I did not file a bug for this, since my exprience with submitting bugs is not so good and for now I prefer to avoid using ant and maven)
(In reply to comment #14) process.destroy really is going to add a race-condition where output may be sitting in a buffer we throw away with destroy before it has been read. I'm sorry that your patch experience has been unpleasant and understand that you don't want to try Ant anymore if you have found a different solution.
(In reply to comment #13) > unfortunately it seems to cause problems at least on Linux No, it doesn't seem to do that. I don't see any difference between test run with or without closing the streams anymore. If anybody can confirm that Ant 1.8.1's <exec> task hangs and it doesn't hang if we apply the patch attached here, please do. Of course it would be great if we had any reproducible testcase.
I've tested my build on Mac, Windows and Linux and all three work fine now using the Ant exec task! Thank you, it has greatly simplified things for me.
(In reply to comment #17) > I've tested my build on Mac, Windows and Linux and all three work fine now > using the Ant exec task! Is this stock Ant 1.8.x (and which one?) or a home-brewed version including my patch? Thanks for testing, BTW.
This was the stock standard 1.8.1
Great, thanks!