Java Forum / General / December 2007
Getting output from exec()’ed program
Hendrik Maryns - 10 Dec 2007 14:25 GMT Hi,
I call a program with ProcessBuilder. From http://www.javaworld.com/javaworld/jw-12-2000/jw-1229-traps.html?page=4 I copied over the class StreamGobbler, to redirect the output. However, sometimes this doesn’t work. I have to do the whole process twice, then it works. I’m out of ideas what the cause could be, and don’t know how to debug this. The SSCCE I extracted does not work at all, in that it doesn’t write anything to the output file. It does write to the console, though.
I read Roedy’s page on exec() and more.
Any ideas mostly appreciated. Classes found below.
==StreamGobbler.java==
import java.io.*;
public class StreamGobbler extends Thread {
private final InputStream is;
private final String type;
private final OutputStream os;
public StreamGobbler(InputStream is, String type, OutputStream redirect) { this.is = is; this.type = type + ">"; os = redirect; }
@Override public void run() { try { PrintWriter pw = null; if (os != null) { pw = new PrintWriter(os); } final InputStreamReader isr = new InputStreamReader(is); final BufferedReader br = new BufferedReader(isr); String line = null; while ((line = br.readLine()) != null) { if (pw != null) { pw.println(line); } System.out.println(type + line); } if (pw != null) { pw.flush(); } } catch (final IOException ioe) { ioe.printStackTrace(); } } }
==MonaCaller.java==
import java.io.*;
public class MonaCaller {
public static void main(String[] args) throws IOException, FileNotFoundException, InterruptedException { final File automFile = new File("test.out"); final ProcessBuilder monaCaller = new ProcessBuilder("echo", "test"); final Process mona = monaCaller.start(); final StreamGobbler monaError = new StreamGobbler( mona.getErrorStream(), "test error", null); final FileOutputStream monaAutomaton = new FileOutputStream(automFile); final StreamGobbler monaOutput = new StreamGobbler(mona .getInputStream(), "test output", monaAutomaton); monaError.start(); monaOutput.start(); mona.waitFor(); monaAutomaton.close(); }
}
TIA, H.
 Signature Hendrik Maryns http://tcl.sfs.uni-tuebingen.de/~hendrik/ ================== http://aouw.org Ask smart questions, get good answers: http://www.catb.org/~esr/faqs/smart-questions.html
Patricia Shanahan - 10 Dec 2007 14:57 GMT ....
> monaError.start(); > monaOutput.start(); > mona.waitFor(); > monaAutomaton.close(); ...
You don't join, or otherwise await completion, for either monaError or monaOutput in this code. Maybe there is data in a buffer that has not yet been processed.
Patricia
Hendrik Maryns - 10 Dec 2007 15:20 GMT Patricia Shanahan schreef:
> .... >> monaError.start(); [quoted text clipped - 6 lines] > monaOutput in this code. Maybe there is data in a buffer that has not > yet been processed. I took me a while to understand what you mean, but now I see: the close() method can come before the thread can write to it. I am very inexperienced with threads, I am afraid. Moving the close into the StreamGobbler class, as Gordon suggested, solves the issue, and makes the waiting unnecessary.
Thanks, H.
 Signature Hendrik Maryns http://tcl.sfs.uni-tuebingen.de/~hendrik/ ================== http://aouw.org Ask smart questions, get good answers: http://www.catb.org/~esr/faqs/smart-questions.html
Gordon Beaton - 10 Dec 2007 15:08 GMT > I call a program with ProcessBuilder. From > http://www.javaworld.com/javaworld/jw-12-2000/jw-1229-traps.html?page=3D4= > > I copied over the class StreamGobbler, to redirect the output. > However, sometimes this doesn=E2=80=99t work. Close the PrintWriter in a finally block in the StreamGobbler, not the OutputStream that it wraps. This is a general rule: always close the outermost stream in the chain.
/gordon
--
Hendrik Maryns - 10 Dec 2007 15:19 GMT Gordon Beaton schreef:
>> I call a program with ProcessBuilder. From >> http://www.javaworld.com/javaworld/jw-12-2000/jw-1229-traps.html?page=3D4= [quoted text clipped - 5 lines] > OutputStream that it wraps. This is a general rule: always close the > outermost stream in the chain. Ah, you’re right, StreamGobbler wraps its os in a PrintWriter. Hm, I should comment on the above website about that. Sadly, somehow the JavaScript does not seem to work.
Thanks, this solves the issue.
H.
 Signature Hendrik Maryns http://tcl.sfs.uni-tuebingen.de/~hendrik/ ================== http://aouw.org Ask smart questions, get good answers: http://www.catb.org/~esr/faqs/smart-questions.html
Daniele Futtorovic - 10 Dec 2007 18:11 GMT > ==StreamGobbler.java== > [quoted text clipped - 39 lines] > } > } @sidenote: This StreamGlobber seems rather bogus to me: - Switching from byte streams to char streams; - Adding a layer of buffering; - Depending on readLine() which might (granted: only exceptionally) introduce problems
All things which shouldn't belong in a InputStream/OutputStream globber. It seems all this was done to be able to fork the output to STDOUT. It would seem more appropriate to me to write a proper StreamFork class. Plus it would be cleaner, IMO, to implement Runnable, rather than extending Thread.
Opinions?
df.
Daniel Pitts - 10 Dec 2007 18:34 GMT >> ==StreamGobbler.java== >> [quoted text clipped - 55 lines] > > df. As for the Runnable vs Thread, it depends if you ever want to run this in a blocking manor. The purpose of this class is to gobble the output so you don't have to wait around.
In general, I prefer that workers (Runnables and Threads) be inner/anonymous classes. Extending thread is actually the preferred approach to change the behavior of a thread.
 Signature Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
Daniele Futtorovic - 10 Dec 2007 19:03 GMT >>> ==StreamGobbler.java== >>> [quoted text clipped - 60 lines] > in a blocking manor. The purpose of this class is to gobble the output > so you don't have to wait around. Implementing Runnable and setting it as a Thread's target, of course.
> In general, I prefer that workers (Runnables and Threads) be > inner/anonymous classes. Extending thread is actually the preferred > approach to change the behavior of a thread. Really? Seems counter-intuitive to me. What /is/ this Object? Is it a Thread? No, it's a globber (or a "pusher" and a pimp ;-)). Being runnable is its characteristic, hence implement the interface.
df.
Hendrik Maryns - 10 Dec 2007 22:49 GMT Daniele Futtorovic schreef:
>> ==StreamGobbler.java== >> [quoted text clipped - 43 lines] > - Switching from byte streams to char streams; > - Adding a layer of buffering; Ok, so this is not a most general class, but those those are wanted behavior. So you’d prefer me to do the wrapping into a buffer outside of this class?
> - Depending on readLine() which might (granted: only exceptionally) > introduce problems Can you give some more explanation on this? Do I understand right that since you think it should use a most general reader, which has no readline(), it should use read() instead?
> All things which shouldn't belong in a InputStream/OutputStream globber. > It seems all this was done to be able to fork the output to STDOUT. It was, but let me stress: not by me.
> It > would seem more appropriate to me to write a proper StreamFork class. > Plus it would be cleaner, IMO, to implement Runnable, rather than > extending Thread. OK, question for debate, but I’ll keep out of that.
H.
 Signature Hendrik Maryns http://tcl.sfs.uni-tuebingen.de/~hendrik/ ================== http://aouw.org Ask smart questions, get good answers: http://www.catb.org/~esr/faqs/smart-questions.html
Daniele Futtorovic - 11 Dec 2007 18:35 GMT > Daniele Futtorovic schreef: >> @sidenote: This StreamGlobber seems rather bogus to me: - Switching [quoted text clipped - 3 lines] > behavior. So you’d prefer me to do the wrapping into a buffer > outside of this class? If at all, yes. I fail to see how buffering has its place in a globber (in the case at hand, it is needed for readLine(), but that's a mistake in itsef, see below).
>> - Depending on readLine() which might (granted: only exceptionally) >> introduce problems > > Can you give some more explanation on this? If your data doesn't contain line feeds, it might take a while. Since you use it to drain a process' output buffer, the globber might even be entirely useless do to this.
> Do I understand right that since you think it should use a most > general reader, which has no readline(), it should use read() > instead? No and yes. No: it shouldn't use /readers/ at all; "reader" as in subclass of java.io.Reader. Readers in this sense are for dealing with character-streams, but the globber's c'tor clearly takes an InputStream and an OutputStream, which deal with data on the byte level. Yes: it should use "read" instead -- the read method of the class InputStream -- with a proper byte buffer.
>> All things which shouldn't belong in a InputStream/OutputStream >> globber. It seems all this was done to be able to fork the output >> to STDOUT. > > It was, but let me stress: not by me. You totally misjudged my post if you think it was directed against you.
I read the page you took your code from. I aknowledged that what I found awkward with this code was due to a specific requirement the author had for it. What I'm criticising isn't so much that the code is wrong, or the like, but that this class isn't, in my opinion, a proper globber.
- The streams shouldn't by wrapped into readers. All kind of funny things might happen if the data weren't actually character data with the proper encoding. Even if they didn't, it's plain unnecessary. - Any kind of buffering shouldn't be done in a globber, but on the streams it is passed - Even with the above set aside, readLine() is still stupid. How about a simple char array?
This globber's code looks more like what one'd write as a quick and dirty hack for a test case. Not something one should publish carelessly (I must say that I haven't read the webpage's text, though -- maybe the author has addressed this) and clearly something I'd discourage from using outside of its narrow scope.
df.
Gordon Beaton - 12 Dec 2007 07:00 GMT >>> - Depending on readLine() which might (granted: only exceptionally) >>> introduce problems [quoted text clipped - 4 lines] > Since you use it to drain a process' output buffer, the globber > might even be entirely useless do to this. Well, no. BufferedReader.readLine() has to read from the stream into a local buffer in order to find the newline, so the process' output buffer is drained properly even if the newline is far away.
/gordon
--
Daniele Futtorovic - 12 Dec 2007 18:19 GMT >>>> - Depending on readLine() which might (granted: only exceptionally) >>>> introduce problems [quoted text clipped - 8 lines] > > /gordon Correct. My apologies to gullible readers for the misinformation (damn!).
df.
Hendrik Maryns - 12 Dec 2007 10:42 GMT Daniele Futtorovic schreef:
>> Daniele Futtorovic schreef: >>> @sidenote: This StreamGlobber seems rather bogus to me: - Switching [quoted text clipped - 16 lines] > you use it to drain a process' output buffer, the globber might even be > entirely useless do to this. Are there processes that produce anything else than text? I acknowledge there might be a problem with encodings, but I don’t expect binary data here.
>> Do I understand right that since you think it should use a most >> general reader, which has no readline(), it should use read() [quoted text clipped - 7 lines] > Yes: it should use "read" instead -- the read method of the class > InputStream -- with a proper byte buffer. To be honest, I am not eager to wrap my head around how this should work. BufferedReader and co. are a convenient way to do what I want, I don’t see why I should bother to look into ‘byte stuff’. I have kept away from that as much as possible.
Maybe I should simply rename the class CharStreamGobbler, would you be satisfied then?
>>> All things which shouldn't belong in a InputStream/OutputStream >>> globber. It seems all this was done to be able to fork the output [quoted text clipped - 17 lines] > - Even with the above set aside, readLine() is still stupid. How about a > simple char array? I’m beginning to understand and acknowledge your points. Thanks.
> This globber's code looks more like what one'd write as a quick and > dirty hack for a test case. Not something one should publish carelessly > (I must say that I haven't read the webpage's text, though -- maybe the > author has addressed this) and clearly something I'd discourage from > using outside of its narrow scope. I’m afraid he doesn’t warn for this. And the code contained other flaws as well (like not waiting for the threads). So I won’t recommend that page anymore.
H.
 Signature Hendrik Maryns http://tcl.sfs.uni-tuebingen.de/~hendrik/ ================== http://aouw.org Ask smart questions, get good answers: http://www.catb.org/~esr/faqs/smart-questions.html
Gordon Beaton - 12 Dec 2007 11:44 GMT > Are there processes that produce anything else than text? I > acknowledge there might be a problem with encodings, but I > don=E2=80=99t expect binar= y data here. Certainly, it's common on Unix.
I remember this being a problem on DOS or Windows, the distinction between "text mode" and "binary mode" when opening files, and that stdin/stdout were typically "text mode". Or something like that.
/gordon
--
Lew - 12 Dec 2007 14:58 GMT >> Are there processes that produce anything else than text? I >> acknowledge there might be a problem with encodings, but I [quoted text clipped - 5 lines] > between "text mode" and "binary mode" when opening files, and that > stdin/stdout were typically "text mode". Or something like that. Different issue. UNIX "text" vs. "binary" is more about linefeeds. Java Reader/Writer vs. Stream is about character encoding.
 Signature Lew
Daniele Futtorovic - 12 Dec 2007 18:42 GMT > Daniele Futtorovic schreef: >>> Daniele Futtorovic schreef: >>>> @sidenote: This StreamGlobber seems rather bogus to me: -
> Are there processes that produce anything else than text? This may or may not be the case; the point is that there are quite a lot of /streams/ which carry something else than text, and that "globbers" have their uses aside from polling an STDOUT/ERR.
Again: my ranting isn't about the adequateness of the piece of code at hand for the task it was used for, but about the fact that this code is, IMHO, a bad example for a "globber".
By the way: the term "globber" might better be reserved for an Object only emptying a stream; what you use here might better be called a "pump", or a "pipepump", or a "pusher"... something along these lines.
>>> Do I understand right that since you think it should use a most >>> general reader, which has no readline(), it should use read() [quoted text clipped - 10 lines] > I don’t see why I should bother to look into ‘byte stuff’. I have > kept away from that as much as possible. Yare yare. You're sure you plan on dealing with computers?
> Maybe I should simply rename the class CharStreamGobbler, would you > be satisfied then? Quite, actually.
If you'd rename the class appropriately; changed the c'tor to take a Reader and a Writer instead of an InputStream and an OutputStream, respectively; removed the wrapping and, accidently, the buffering; used a char array instead of the line-String; possibly gave options for closing and cleaning up a thought: I'd be really satisfied (I will refrain from purring, promise).
Yours, etc.
Daniele Futtorovic - 12 Dec 2007 18:50 GMT > By the way: the term "globber" might better be reserved for > an Object only emptying a stream; what you use here might better be > called a "pump", or a "pipepump", or a "pusher"... something along these > lines. Just noticed that this whole thread through I've assumed your class had been named "globber", when it was actually called "gobbler". Sorry about that.
df.
Roedy Green - 12 Dec 2007 15:59 GMT On Mon, 10 Dec 2007 15:25:30 +0100, Hendrik Maryns <gtw37bn02@sneakemail.com> wrote, quoted or indirectly quoted someone who said :
>I read Roedys page on exec() and more. Here is a different sort of approach you could have used to crack your problem:
Run my SSCCE program at http://mindprod.com/jgloss/exec.html
If it does not work, complain to me. That is MY problem to sort out. The advantage is it must have worked at some point at least on my machine, so it should not be too hard to get it working on yours and it is really not your responsibility to solve.
Now gradually modify my program to your needs. At each stage run it to make sure it is still working. If it stops working, revert to the previous version and perhaps ask for help on that particular point, or just think, or try something slightly different, or try debugging/tracing.
 Signature Roedy Green Canadian Mind Products The Java Glossary http://mindprod.com
Hendrik Maryns - 13 Dec 2007 10:39 GMT Roedy Green schreef:
> On Mon, 10 Dec 2007 15:25:30 +0100, Hendrik Maryns > <gtw37bn02@sneakemail.com> wrote, quoted or indirectly quoted someone [quoted text clipped - 17 lines] > just think, or try something slightly different, or try > debugging/tracing. Good idea. I’ll think about it if in the future I need something similar again.
Cheers, H.
 Signature Hendrik Maryns http://tcl.sfs.uni-tuebingen.de/~hendrik/ ================== http://aouw.org Ask smart questions, get good answers: http://www.catb.org/~esr/faqs/smart-questions.html
Free MagazinesGet these publications absolutely FREE for up to 12 months. There are no hidden fees and no obligation. Simply choose a title, complete the application form and submit it. Read more ...
|
|
|