File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
The moose likes Threads and Synchronization and the fly likes race condition with Runtime.exec()? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Spring in Action this week in the Spring forum!
JavaRanch » Java Forums » Java » Threads and Synchronization
Bookmark "race condition with Runtime.exec()?" Watch "race condition with Runtime.exec()?" New topic
Author

race condition with Runtime.exec()?

B Mayes
Ranch Hand

Joined: Apr 28, 2010
Posts: 47

I'm wrapping Runtime.exec() with a utility function that I created to execute an external process. I have read many articles on the pitfalls of Runtime.exec() and thought I had everything working, but I just recently discovered that while invoking external processes seems to work almost everywhere in my code, it's still failing in one spot. We basically have a C program that gets invoked in some situations, and on Unix/Linux the call is wrapped with a shell script. When our web app starts, it attempts to locate either the "fromdos" command or the "dos2unix" command to make sure that the shell script used has Unix EOL characters (if the file ends with DOS EOL characters then it fails to run properly). It also sets the script to be executable by running chmod +x on it but that part seems to be working.

It appears that my calls to the commands "which dos2unix" and "which fromdos" are not working. If I set a breakpoint and step through the debugger however -- then it returns the appropriate output and it all works great. This suggests that I am doing something wrong with the threads and there is a race condition.

I am using stream gobblers for stdout/stderr, join the threads, and feel like everything should be working. Below is an SSCCE (maybe not quite as small as I can make it) that fails on my system (64-bit Ubuntu 12.10 with Sun Java 1.6.0_26). Any idea why this doesn't work? Is it because which is a built-in or something? I tried changing the calls to use an argument array like but it still doesn't work. Maybe I should just finally move everything to use the ProcessBuilder class instead of Runtime.exec()? Any help is appreciated!

Here is an example of running it on my system:






And here is the entire SSCCE.java file. Try compiling it and you may/may not get the same output that I do above:


Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

The StreamGobbler uses while(br.ready()) to perform the 'get data from stream' loop. This is not going to work. The br.ready() returns true when there is data in the buffer to be read. When the BufferedReader first starts and before anything is sent to it, the br.ready() will return false - indicating your StreamGobbler should shutdown before it ever started. You can't rely on that. In fact, you can't really rely on anything directly related to the BufferReader to tell you when you need to shutdown. The best thing to do is to listen to an external stop signal which (1) sets a flag to stop the gobbler, and (2) listen to interruption. You would then start the gobblers, wait for the process to complete, stop the gobblers.

I have an implementation that will work better for you, but not on me. So I will post it once I get to the machine with the code.


Steve
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

Okay, here it is. Upon further reflection, I wonder if the stop() is required at all... Will have to test that out:

B Mayes
Ranch Hand

Joined: Apr 28, 2010
Posts: 47

Steve Luke wrote:The StreamGobbler uses while(br.ready()) to perform the 'get data from stream' loop. This is not going to work. The br.ready() returns true when there is data in the buffer to be read. When the BufferedReader first starts and before anything is sent to it, the br.ready() will return false - indicating your StreamGobbler should shutdown before it ever started. You can't rely on that. In fact, you can't really rely on anything directly related to the BufferReader to tell you when you need to shutdown.


Man if that is true then the now-classic article "When Runtime.exec() won't" is completely bogus, and thus lots of people should be hosed by this. I'm sure you have read it but here is link for those who haven't:

http://www.javaworld.com/jw-12-2000/jw-1229-traps.html


I think their example is flawed in that it doesn't join the gobbler threads...but I see what you're saying. It's possible those threads could get scheduled and started prior to the "main" thread executing. So even though you call join, if those threads read the input stream before any data was written there, they would simply exit and calling join is quite ineffective at that point.

Thanks for the replies...I'll take a look and try some things out tomorrow if I get the chance.
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

B Mayes wrote:
Steve Luke wrote:The StreamGobbler uses while(br.ready()) to perform the 'get data from stream' loop. This is not going to work. The br.ready() returns true when there is data in the buffer to be read. When the BufferedReader first starts and before anything is sent to it, the br.ready() will return false - indicating your StreamGobbler should shutdown before it ever started. You can't rely on that. In fact, you can't really rely on anything directly related to the BufferReader to tell you when you need to shutdown.


Man if that is true then the now-classic article "When Runtime.exec() won't" is completely bogus, and thus lots of people should be hosed by this. I'm sure you have read it but here is link for those who haven't:

http://www.javaworld.com/jw-12-2000/jw-1229-traps.html

I wouldn't say it is bogus. First, he doesn't use br.ready() he uses br.readLine() != null. That works as long as you assume the gobblers should run until the process kills the streams. That is probably pretty safe, but a forced stop puts it under more control, in my opinion. And lets you close the streams on your end when you are done with them. I personally think a forced stop is cleaner, but but I don't fault the sample code because
1) it is an article with example code, not attempting to be production quality
2) I have never measured or experienced a problem with waiting for the process to close its streams, it just feels wrong to me.


I think their example is flawed in that it doesn't join the gobbler threads...

There is no real need to join the gobbler threads. Do you really need to block until the gobblers are done? Isn't knowing the process is done good enough? Especially if you can tell the gobbler to end.

but I see what you're saying. It's possible those threads could get scheduled and started prior to the "main" thread executing.

No, they can't run before main, because main thread creates the gobbler threads, so there is a happens-before relationship between code executed in the main thread (prior to start() being called) and code in the worker thread. But the gobbler threads can easily reach the br.ready() check before the process has put data in the streams.

So even though you call join, if those threads read the input stream before any data was written there, they would simply exit and calling join is quite ineffective at that point.

Thanks for the replies...I'll take a look and try some things out tomorrow if I get the chance.
B Mayes
Ranch Hand

Joined: Apr 28, 2010
Posts: 47

<---- Schooled.

Wow, great info.


Steve Luke wrote:
I wouldn't say it is bogus. First, he doesn't use br.ready() he uses br.readLine() != null. That works as long as you assume the gobblers should run until the process kills the streams. That is probably pretty safe, but a forced stop puts it under more control, in my opinion. And lets you close the streams on your end when you are done with them. I personally think a forced stop is cleaner, but but I don't fault the sample code because
1) it is an article with example code, not attempting to be production quality
2) I have never measured or experienced a problem with waiting for the process to close its streams, it just feels wrong to me.

Valid points.


Steve Luke wrote:
There is no real need to join the gobbler threads. Do you really need to block until the gobblers are done? Isn't knowing the process is done good enough? Especially if you can tell the gobbler to end.

I guess my whole idea behind joining the threads was to ensure that the main thread blocks until the spawned threads complete. On the surface I assumed that a successful completion would always indicate that the streams had been fully consumed, but as you have pointed out that may not always be the case when using br.ready(). In this case, the meaning of the word "consumed" is the tricky part.


Steve Luke wrote:
No, they can't run before main, because main thread creates the gobbler threads, so there is a happens-before relationship between code executed in the main thread (prior to start() being called) and code in the worker thread. But the gobbler threads can easily reach the br.ready() check before the process has put data in the streams.

Indeed you're correct. I followed the basic concept though. Thanks for clarifying the finer details.



Now I'm going to go hang in my head in shame for a bit before I go to bed. And maybe google "Steve Luke thread tutorials" as well. ;)


As I said, I will give this a shot tomorrow if I have the time. Right now the only thing that seems to be affected is that the dos2unix/fromdos command isn't being executed on a shell script at system startup. Apparently our clients have been running it manually each time they deploy a new war file for years anyway. I'm just trying to automate the process for them and ran into this snag. Cheers.
B Mayes
Ranch Hand

Joined: Apr 28, 2010
Posts: 47

Steve,

I finally got a chance to work on this today and it appears to be working great. I tweaked a few things but otherwise I am essentially using it as-is. Code will be committed shortly and will go through some additional testing to verify that it's working but just on my own system it has been working every time. Thank you so much!
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

That's great it is working, thanks for the feedback :-)
B Mayes
Ranch Hand

Joined: Apr 28, 2010
Posts: 47

Steve Luke wrote:That's great it is working, thanks for the feedback :-)


Oops...maybe I spoke too soon. Below is a new SSCCE that just runs the uname -m command 1000 times and counts up the number of times the output returned is empty. It happens rather infrequently to me but the problem is that it does still sometimes happen, although during my web app initialization it seems to fail much more frequently than it actually works. In this SSCCE the max number of times it failed after running it a dozen or so times was 4.

Here is what I have. I have stripped out all of the javadoc to condense things, though I did give you and Michael Daconta credit for the StreamGobbler class. ;)

Did I screw something up when I made the tweaks to the class?


Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

Have you checked the error stream output and the return value from the called application to see if the error was internal and not a problem with you Java code?
B Mayes
Ranch Hand

Joined: Apr 28, 2010
Posts: 47

Steve Luke wrote:Have you checked the error stream output and the return value from the called application to see if the error was internal and not a problem with you Java code?


I'm not 100% sure I follow exactly what you want to look at, but I changed up the main method to print more information when a failing call occurs and also to print a successful call like this:





Here is the output that I got. Note that the return value for all "bad" calls was always zero...


$ java SSCCE
Counted 30 time(s) where the output was empty

[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]
[0,,]


Valid calls look like this: [0,x86_64
,]





I'm at home right now and don't really feel like rebooting into an old WinXP partition (plus I need to start getting ready for work and the kids ready for school, etc.). I will try this code on a Win7 VM when I get to the office and change the call to just run a directory listing. For Windows I believe the syntax would be something like this if anybody else wants to run it:

Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

Hmm... So the returned text you expect is pretty minimal. It could be that the output buffer Windows uses is not being flushed before you call the stop() method because it never gets filled. So this could be a situation where my preference to control the stop versus letting the process die (and join the gobblers) is not good.


Try adding a join on the gobbler threads:

So it replaces the stop with a join, assuming the process will flush its buffer before shutting down. I wish I could test for you, but no time over the next few days.
B Mayes
Ranch Hand

Joined: Apr 28, 2010
Posts: 47

Steve Luke wrote:Hmm... So the returned text you expect is pretty minimal. It could be that the output buffer Windows uses is not being flushed before you call the stop() method because it never gets filled.

I think you mean the output buffer in Linux instead. I'm pretty sure the uname command doesn't work on Windows. ;)


Steve Luke wrote:
So this could be a situation where my preference to control the stop versus letting the process die (and join the gobblers) is not good.

It appears so. I guess I was right about joining the gobblers after all...at least I was right about something. I modified it to join the gobblers and presto it works. I ran it a few times and got 0 failures. So then I stuck it in a big bash loop and ran it 100 times:



$ for i in $(seq 1 100)
> do
> java SSCCE
> done
Counted 0 time(s) where the output was empty
Counted 0 time(s) where the output was empty
Counted 0 time(s) where the output was empty
Counted 0 time(s) where the output was empty
Counted 0 time(s) where the output was empty
...



You get the idea. IT WORKS!! Hopefully testing it 100k times and getting output in every single one is sufficient. I'm convinced at this point anyway.


Just FYI:

Steve Luke wrote:
/* completely untested example... maybe add a timeout, and better return or error handling or something... */

While I didn't show it for this SSCCE, I actually did wrap my process execution with a TimerTask as suggested here:
http://kylecartmell.com/?p=9


He also discusses Sun bug 6420270 so I call Thread.interrupted() to clear the interrupt flag as suggested. This bug isn't marked as fixed until Java 7 and most of our customers are on Java 6. I actually discovered one of our clients is still running Java 5...

Back to the timer though. My actual code has a default wait time of 5 seconds, and then the method is overloaded to have a single argument (the command), or 2 arguments (command and a timeout). If the timeout is null it simply falls back on the default. Yes, I use an int rather than long, but I cannot imagine anyone wanting to run a process longer than Integer.MAX_INT seconds anyway. That equates to like 596 hours or roughly 25 days! In any case, if the external process isn't done after the timeout expires then we simply interrupt the thread so that the darn thing doesn't block forever (perhaps due to an infinite loop). So here is the "complete" version of executeProcess():





Your input has been incredibly useful here and I really appreciate it. If we happen to meet up on the street then I owe you a beer, without question.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: race condition with Runtime.exec()?