1

I'm reading a file in via apache.commons.FtpClient.

This works fine 99.9% of the time but sometimes it just dies in the read() method...

InputStream inStream = ftp.retrieveFileStream(path + file.getName());
String fileAsString = "";

if(inStream == null){
    return;
}
while((c = inStream.read()) != -1){ //this is where the code sometimes just hangs               
   fileAsString += Character.valueOf((char)c);

}

My question is what is the most reliable way to protect against this locking up the system indefinitely. Should I be setting a timer in a separate thread? Or is there a simpler way to do it?

starblue
  • 55,348
  • 14
  • 97
  • 151
Yevgeny Simkin
  • 27,946
  • 39
  • 137
  • 236

2 Answers2

4

If your code hangs it means your FTP server has not sent the entire file. You can use a Timer, but I believe FtpClient allows you to set a timeout.

BTW: the way you read the file is very inefficient. If your file is larger than a few K it will use increasing amounts of CPU.

You are creating a Character from a byte (which is a bad idea in itself) and a String object for every byte in the file.

I suggest using the copy method provided or the one which comes with commons-io library to copy the data to a ByteArrayInputStream.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • plus 1 for pointing out the char/byte+string issue. This should be a StringBuilder.append at least. – John Gardner Feb 28 '09 at 08:23
  • and InputStream.read(byte[] b, int off, int len) should be used as well. So while this post is not the actual answer, I would say it provides the most help to the OP - +1 from me as well. – Kevin Day Mar 01 '09 at 05:08
  • I agree InputStream.read(byte[] b, int off, int len) should be use, but this had been mentioned by one of the 3 answers given, I can only see two at the moment. ?? – Peter Lawrey Mar 01 '09 at 08:03
  • thanks guys. Point taken on the read methodology. However, that's not the problem... the connection goes into lala land at file.getName() but only rarely... it's actually fairly hard to replicate the problem. I'm guessing some sort of ftp wackiness? – Yevgeny Simkin Mar 01 '09 at 16:35
1

Just from a quick look at the docs, if you did...

while (inStream.available() > 0 && (c = inStream.read()) != -1)

It seems like it would double check that you can read without blocking before you actually read. I'm not certain on this though.

Drew
  • 15,158
  • 17
  • 68
  • 77
  • I like this solution, though I suspect there's still a chance that between the check of available and the read() the connection can go stale somehow... I realize this is very unlikely, but it can happen, right? – Yevgeny Simkin Feb 27 '09 at 22:59
  • If you have a slow connection available() will return 0 either at the start of the file or some way before the end. – Peter Lawrey Feb 28 '09 at 07:16
  • Unless there's something seriously wrong in FtpClient, this can't be the real solution. read() should block only if available() returns 0, so instead of waiting in read() you'll be waiting in this loop and eating a lot more CPU. – Dan Berindei Mar 12 '11 at 13:14
  • Isn't reading one byte after another a bit inefficient? I think it's better to use a byte array. – android developer Jun 09 '20 at 07:53