Problem reading large file

[ohaya]

Hi,

First of all, my apologies if any of you see a similar post in
comp.lang.java. I was rushing, and didn't realize that the
comp.lang.java newsgroup might be deprecated. Hopefully, this is the
correct group now. Also, I don't have access to a news server
directly where I am now, so I'm posting via Google Groups...


Jim

Hi,

I'm a real newbie, but have been asked to try to fix a problem in one
of our JSP pages that is suppose to read in a text file and display
it.

From my testing thus far, it appears this page is somehow hanging when
relatively large file is used.

My original intent was to try to just add a check for file size, and
error out somehow if the file was "too" large, but in looking at the
code, I'm not quite sure what might cause it to hang even with large
files.

Here's a portion of the code, with some of the code, etc. removed for
clarity and with some comments (and hoping that I don't make any
typos):

..
..
String finalFileData = null;
..
..
java.io.File file = new java.io.File (filePath); // filePath is a
String, containing full path+name
..
..
try {
java.io.FileReader freader = null;
try {
freader = new java.io.FileReader(file); // create FileReader
StringBuffer fileData = new StringBuffer((int) file.length());
char[] chars = new char[SOME_BUFFER_SIZE];
int c = freader.read(chars); // read initial buffer into 'chars'
while (c > 0) {
fileData.append(chars, 0, c); // append string from 'chars' to
'fileData'
c = freader.read(chars); // read next buffer into 'chars'
}
finalFileData = fileData.toString();
} finally {
if (freader != null)
freader.close();
}
} catch (Exception e) {
finalFileData = null;
}
..
..

When I looked at the code above, it seems like the the line where the
'fileData' StringBuffer is created is creating a StringBuffer that is
size-limited to the maximum of an int in Java. I'm assuming that this
limit is 'Integer.MAX_VALUE'. Is that correct?

Assuming that that's (that the fileData size is set that way) the
case, it looks like the 'while' loop is NOT size-limiting the reads
into the fileData StringBuffer, so I'm guessing that that is what is
the problem.

Does this seem about right?

Then, I don't understand why the page is just hanging, and not
generating an exception. Shouldn't the try (and catch, at the end)
cause an exception error)?

As I mentioned above, I'm kind of a real newbie with Java/JSP, and
having to fix a problem with code from someone else, so I hope that
this is not too stupid question.

Thanks in advance,
Jim

 

[Chris Uppal]

When I looked at the code above, it seems like the the line where the
'fileData' StringBuffer is created is creating a StringBuffer that is
size-limited to the maximum of an int in Java. I'm assuming that this
limit is 'Integer.MAX_VALUE'. Is that correct?

Well, I think that a cast of a long to an int just masks the long down to
32-bits rather than truncates. If that result in a negative number then the
StringBuffer constructor should throw an exception. Otherwise it shouldn't
make a functional difference (though it's obviously a programming error)
because the StringBuffer will expand as needed anyway.

Assuming that that's (that the fileData size is set that way) the
case, it looks like the 'while' loop is NOT size-limiting the reads
into the fileData StringBuffer, so I'm guessing that that is what is
the problem.

Looks like it. As far as I can tell the StrngBuffer (the JDK1.4.2
implementation) is not properly protected against signed integer wrap-around as
its buffer size approaches Integer.MAX_VALUE (there is some protection, but I
don't think there's enough). I'm a little surprised that that doesn't cause
ArrayIndexOutOfBoundsExceptions as it uses negative indexes into its internal
buffer, but I haven't really followed the code through.

As I mentioned above, I'm kind of a real newbie with Java/JSP, and
having to fix a problem with code from someone else, so I hope that
this is not too stupid question.

Not at all a stupid question. In fact it's the answer that's stupid ;-)

-- chris

 

[ohaya]

Chris,

Comments below, interspersed...

..
..
snip
..
..

Looks like it. As far as I can tell the StrngBuffer (the JDK1.4.2
implementation) is not properly protected against signed integer wrap-around as
its buffer size approaches Integer.MAX_VALUE (there is some protection, but I
don't think there's enough). I'm a little surprised that that doesn't cause
ArrayIndexOutOfBoundsExceptions as it uses negative indexes into its internal
buffer, but I haven't really followed the code through.

Maybe I've been staring at this code too long, but it looks like the
innermost 'try' has a 'finally' clause, but not 'catch' clause? Is this
a (possible) problem?

Not at all a stupid question. In fact it's the answer that's stupid ;-)

No, Chris, I do appreciate your response!

Jim

 

[Chris Uppal]

Maybe I've been staring at this code too long, but it looks like the
innermost 'try' has a 'finally' clause, but not 'catch' clause? Is this
a (possible) problem?

Shouldn't be. It's of the form

try
{
try
{
...code...
}
finally
{
...more code...
}
}
catch (Exception)
{
...not much code here...
}

So any exceptions thrown from <...code...> should get caught by the handler.

To be honest, I have no idea why this page is "hanging". If you are handling
files with sizes over 2**31, then the code will certainly have problems with
integer wrap-around, and it's rather difficult to guess what effect that would
have. Among other things, it depends on how the specific Java installation
would react as sizes /approached/ 2**31 -- on my machine (which is just a
Windows laptop) it'd certainly fail with an out-of-memory error long before
that point was reached. So another possibility is that the code /is/ throwing
exceptions of some sort or other, and that they are being caught, leaving
"finalFileData" == null, and that it is some later processing that is causing
the "hang" when it sees the null.

BTW, I hadn't noticed this before, but the handler (unless you've deleted its
contents for brevity) catches all Exceptions and largely ignores them. It's
unlikely that that is what it "should" be doing. For one thing it's only
rarely appropriate to catch blanket exceptions like that (though this /might/
be one of those cases). For another thing, it's even more rarely appropriate
to discard an exception without at least logging it (and this does not appear
to be one of those cases)

-- chris