Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Error opening file.

0 views
Skip to first unread message

bcr666

unread,
Nov 14, 2007, 11:31:41 PM11/14/07
to
I am getting the following stack trace:

java.io.IOException: Read error
at java.io.FileInputStream.readBytes(Native Method)
at java.io.FileInputStream.read(FileInputStream.java:177)
at net.bcr666.hexwriter.Loader.run(Loader.java:35)

This is the code from the class
import java.util.*;

import javax.swing.event.*;
import java.io.IOException;
import javax.swing.JOptionPane;
import java.io.InputStream;
import javax.swing.SwingUtilities;

public class Loader extends Thread {
transient Vector changeListeners = null;
InputStream inputStream = null;
HexWriter parent = null;
Vector data = new Vector();

public Loader(InputStream inputStream, HexWriter parent) {
this.inputStream = inputStream;
this.parent = parent;
}

public byte[] getData() {
if (data.size() == 0) {
return null;
}
return (byte[])data.get(0);
}

public void run() {
byte[] input = new byte[16];
int status = 0;

do {
try {
status = inputStream.read(input);
} catch (IOException ex) {
JOptionPane.showMessageDialog(parent, "Error reading
file.");
ex.printStackTrace();
fireStateChanged(new ChangeEvent(this));
return;
} finally {
try { inputStream.close(); } catch (IOException ex) {}
}
if (status > 0) {
data.add(input);
fireStateChanged(new ChangeEvent(this));
}
} while (status > -1);

try {
fireStateChanged(new ChangeEvent(this));
inputStream.close();
} catch (Exception ex) {}
}

public synchronized void addChangeListener(ChangeListener listener)
{
if (changeListeners == null) {
changeListeners = new Vector();
}
if (!changeListeners.contains(listener)) {
changeListeners.add(listener);
}
}

public synchronized void removeChangeListener(ChangeListener
listener) {
if (changeListeners == null) {
return;
}
if (changeListeners.contains(listener)) {
changeListeners.remove(listener);
}
if (changeListeners.size() == 0) {
changeListeners = null;
}
}

protected void fireStateChanged(final ChangeEvent e) {
if (changeListeners != null) {
final Vector listeners = changeListeners;
int count = listeners.size();
for (int i = 0; i < count; i++) {
final int j = i;
SwingUtilities.invokeLater(new Runnable() {
public void run() {
( (ChangeListener)
listeners.elementAt(j)).stateChanged(e);
}
});
}
}
}
}

The InputStream is passed in from another class and is guaranteed to
be open. I get the same error no matter what file I try to open. Am
I doing something wrong?

Andrew Thompson

unread,
Nov 14, 2007, 11:47:21 PM11/14/07
to
bcr666 wrote:
..
>java.io.IOException: Read error
..

>This is the code from the class

'The' class? But it seems..

>The InputStream is passed in from another class ..

.another class is involved, and the problem might be
there.

>..and is guaranteed to be open.

LOL 'Famous last words'..

>...I get the same error no matter what file I try to open. Am
>I doing something wrong?

Avoiding preparing an SSCCE?

--
Andrew Thompson
http://www.athompson.info/andrew/

Message posted via JavaKB.com
http://www.javakb.com/Uwe/Forums.aspx/java-general/200711/1

Knute Johnson

unread,
Nov 15, 2007, 12:01:56 AM11/15/07
to

You are not guaranteed to read all 16 bytes, or any bytes for that
matter. Why would you open the stream in another class and read it in
this one? Why does getData() only return the first element? You could
possibly read more than one but never retrieve it.

This is really a mess and without the other pieces it is very difficult
to figure out what your problem is. The error message shows you the
line number of the error. What program line is that?

Why don't you either tell us what you are really trying to do or provide
an SSCCE.

--

Knute Johnson
email s/nospam/knute/

Esmond Pitt

unread,
Nov 15, 2007, 1:49:59 AM11/15/07
to
bcr666 wrote:

> I am getting the following stack trace:
>
> java.io.IOException: Read error
> at java.io.FileInputStream.readBytes(Native Method)
> at java.io.FileInputStream.read(FileInputStream.java:177)
> at net.bcr666.hexwriter.Loader.run(Loader.java:35)
>

> The InputStream is passed in from another class and is guaranteed to
> be open. I get the same error no matter what file I try to open.

First of all, that's not an error opening a file. It's an I/O error
reading the FileInputStream. Looks like a disk error to me, bad luck.

bcr666

unread,
Nov 15, 2007, 8:58:15 AM11/15/07
to
On Nov 14, 11:01 pm, Knute Johnson <nos...@rabbitbrush.frazmtn.com>
wrote:

> Why would you open the stream in another class and read it in
> this one?

This class is just for reading the data from the file. The file may
be large, and I am attempting to read it in chunks so the user doesn't
have to wait for an eternity for the GUI to update. So the file is
checked to exist, and then it is opened in the GUI, after which the
reading of the file is offloaded here.

> Why does getData() only return the first element? You could
> possibly read more than one but never retrieve it.

Notice that the 1st element is retrieved, and also removed from the
Vector, so the next call to getData() will actually return the next
value. The theory here is that after every 16 bytes are read in, they
are placed into a FIFO stack, listeners are then notified that there
is new data waiting for pickup, listeners (GUI) can then pop the first
item off of the stack, and display it on the screen.

I think I've tracked the problem down to the finally statement. I
think it is closing the stream too early. I will move it into the
catch tonight and see how that goes.

Knute Johnson

unread,
Nov 15, 2007, 12:56:26 PM11/15/07
to
bcr666 wrote:
> On Nov 14, 11:01 pm, Knute Johnson <nos...@rabbitbrush.frazmtn.com>
> wrote:
>> Why would you open the stream in another class and read it in
>> this one?
>
> This class is just for reading the data from the file. The file may
> be large, and I am attempting to read it in chunks so the user doesn't
> have to wait for an eternity for the GUI to update. So the file is
> checked to exist, and then it is opened in the GUI, after which the
> reading of the file is offloaded here.

Why don't you just use a method rather than a class? All this
conversion from bytes to Vectors and back is going to be very expensive.
Why don't you just read the file, buffer it if you want? Your
reading method is very dodgy. It may or may not read anything on any
particular try. And I assume you have another class that uses this data?

>> Why does getData() only return the first element? You could
>> possibly read more than one but never retrieve it.
>
> Notice that the 1st element is retrieved, and also removed from the
> Vector, so the next call to getData() will actually return the next
> value. The theory here is that after every 16 bytes are read in, they
> are placed into a FIFO stack, listeners are then notified that there
> is new data waiting for pickup, listeners (GUI) can then pop the first
> item off of the stack, and display it on the screen.

Vector.get() does not remove that element from the Vector.

> I think I've tracked the problem down to the finally statement. I
> think it is closing the stream too early. I will move it into the
> catch tonight and see how that goes.

That's a problem I missed when I first looked at it. It will only
execute the try block once and then the finally block will close the
stream and it will throw an exception when you try to read then next
group of bytes.

You need to read your data and process it. Storing byte data in Vectors
doesn't make a lot of sense. If you don't want to hold up your GUI
while you read the data, you need to read it in another thread.

bcr666

unread,
Nov 16, 2007, 8:52:51 AM11/16/07
to
On Nov 15, 11:56 am, Knute Johnson <nos...@rabbitbrush.frazmtn.com>
wrote:

> > This class is just for reading the data from the file. The file may
> > be large, and I am attempting to read it in chunks so the user doesn't
> > have to wait for an eternity for the GUI to update. So the file is
> > checked to exist, and then it is opened in the GUI, after which the
> > reading of the file is offloaded here.
>
> Why don't you just use a method rather than a class? All this
> conversion from bytes to Vectors and back is going to be very expensive.
> Why don't you just read the file, buffer it if you want?

How do I buffer it without putting it into something?

>
> >> Why does getData() only return the first element? You could
> >> possibly read more than one but never retrieve it.
>
> > Notice that the 1st element is retrieved, and also removed from the
> > Vector, so the next call to getData() will actually return the next
> > value. The theory here is that after every 16 bytes are read in, they
> > are placed into a FIFO stack, listeners are then notified that there
> > is new data waiting for pickup, listeners (GUI) can then pop the first
> > item off of the stack, and display it on the screen.
>
> Vector.get() does not remove that element from the Vector.

I'm sorry, I came across that bug and fixed it, I didn't notice it was
included in the above code. I now have a data.remove(0) in there too.

> You need to read your data and process it. Storing byte data in Vectors
> doesn't make a lot of sense. If you don't want to hold up your GUI
> while you read the data, you need to read it in another thread.


Notice that this class extends thread.

The general principle is that the user clicks File>Open and selects a
file from the dialog. The GUI checks to make sure the file exists,
and attempts to open a stream from it. If all has gone well, the GUI
creates an instance of this loader thread, passes in the stream,
registers itself as a listener, and starts the thread. This class
then reads the data from the file in 16 byte increments, stores each
increment in the data vector, and notifies the GUI that there is data
waiting. The GUI can then come at its own leisure and grab the first
element off of the data vector and put it on the screen after some
processing.

Knute Johnson

unread,
Nov 16, 2007, 1:12:18 PM11/16/07
to
bcr666 wrote:
> On Nov 15, 11:56 am, Knute Johnson <nos...@rabbitbrush.frazmtn.com>
> wrote:
>>> This class is just for reading the data from the file. The file may
>>> be large, and I am attempting to read it in chunks so the user doesn't
>>> have to wait for an eternity for the GUI to update. So the file is
>>> checked to exist, and then it is opened in the GUI, after which the
>>> reading of the file is offloaded here.
>> Why don't you just use a method rather than a class? All this
>> conversion from bytes to Vectors and back is going to be very expensive.
>> Why don't you just read the file, buffer it if you want?
>
> How do I buffer it without putting it into something?

It would make the most sense to process your data as it is read.

>>>> Why does getData() only return the first element? You could
>>>> possibly read more than one but never retrieve it.
>>> Notice that the 1st element is retrieved, and also removed from the
>>> Vector, so the next call to getData() will actually return the next
>>> value. The theory here is that after every 16 bytes are read in, they
>>> are placed into a FIFO stack, listeners are then notified that there
>>> is new data waiting for pickup, listeners (GUI) can then pop the first
>>> item off of the stack, and display it on the screen.
>> Vector.get() does not remove that element from the Vector.
>
> I'm sorry, I came across that bug and fixed it, I didn't notice it was
> included in the above code. I now have a data.remove(0) in there too.

You don't need both, only the remove(). I returns the element too.

>> You need to read your data and process it. Storing byte data in Vectors
>> doesn't make a lot of sense. If you don't want to hold up your GUI
>> while you read the data, you need to read it in another thread.
>
>
> Notice that this class extends thread.
>
> The general principle is that the user clicks File>Open and selects a
> file from the dialog. The GUI checks to make sure the file exists,
> and attempts to open a stream from it. If all has gone well, the GUI
> creates an instance of this loader thread, passes in the stream,
> registers itself as a listener, and starts the thread. This class
> then reads the data from the file in 16 byte increments, stores each
> increment in the data vector, and notifies the GUI that there is data
> waiting. The GUI can then come at its own leisure and grab the first
> element off of the data vector and put it on the screen after some
> processing.
>

If this is some exercise in how to make it more complicated than have
fun. Otherwise, read the data, process it if you need to, and display
it. I couldn't have come up with a more inefficient or complicated way
to do this if I had thought about it.

What sort of data is it actually? Why can you not just read the file
and display the data? How much data is there?

Roedy Green

unread,
Nov 17, 2007, 6:02:14 AM11/17/07
to
On Wed, 14 Nov 2007 20:31:41 -0800 (PST), bcr666 <bcr...@gmail.com>
wrote, quoted or indirectly quoted someone who said :

> try {
> status = inputStream.read(input);
> } catch (IOException ex) {

What's a bit peculiar is the logical place your exception would occur
is here. You did not tell us the line numbers or the lines.. Yet you
catch the exception.

You are trying to read 16 bytes. Are there 16 raw bytes in the
inputStream to read?

Your variable "status" is misnamed. It actually bytesRead. You don't
necessarily get all you ask for.
--
Roedy Green Canadian Mind Products
The Java Glossary
http://mindprod.com

Curt Welch

unread,
Nov 19, 2007, 3:20:25 AM11/19/07
to
bcr666 <bcr...@gmail.com> wrote:

> public void run() {
> byte[] input = new byte[16];
> int status = 0;
>
> do {
> try {
> status = inputStream.read(input);
> } catch (IOException ex) {
> JOptionPane.showMessageDialog(parent, "Error reading
> file.");
> ex.printStackTrace();
> fireStateChanged(new ChangeEvent(this));
> return;
> } finally {
> try { inputStream.close(); } catch (IOException ex) {}

As you seemed to figure out, this was your bug. The fianlly {} clause will
get executed every time through the loop whether an exception was thrown or
not (and whether you return or break inside the try{}catch{} so you close
the stream after the the first read and it throws an exception on the
second read.

> }
> if (status > 0) {
> data.add(input);

There's another bug here I didn't see mentioned in the other messages.
(maybe I missed it?)

You use the same byte array to read each chunk of data, and you keep
putting the same byte array into the vector. All you are doing is putting
a pointer to the single byte array into the vector. So instead of putting
100 buffers with 16 bytes each into it, you are putting 100 pointers to the
single 16 byte buffer into the vector.

The other bug, which was pointed out, is that you can't assume it will read
16 bytes each time, so when the rest of your application pulls these
buffers out and assumes it has 16 bytes of data in it, it might get old
data. In practice, if you are reading a disk file, you will get 16 bytes
for all but the last read. But the last read will be short if the file
length is not a multiple of 16 bytes and your reading code won't know this.

One way to fix both of these problems is to allocate a new byte buffer for
each read and make the size of the new byte array match what you actually
read:

data.add(Arrays.copyOf(input, status));

If you GUI code is going to do a loop like:

while ((data = getData()) != null)
updateGUI(data);

Then you might have another problem. Your read thread can read and put a
ton of data into the vector before the the GUI is ever called, and when it
does get called, it's going to have so much work to do that it could lock
up your GUI and defeat the whole reason you are using this background
thread in the first place. This would be seen when reading the same large
file twice so that the second time you read it, it's all cached in memory
by the OS so your read thread will be very fast.

You need to flow control the write thread so that it has a limit on how
much it will put into the vector. There are many different ways to do
that, but you could for example have the file read thread wait on the
Vector when it gets above a given size, and have your getData() method do a
notify on the Vector every time data is taken out.

I would strongly advise using larger buffers for each read as well. Firing
an invokeLater() for each 16 bytes read is going to be very slow and
painful.

If you want the screen to update for each 16 bytes read (one line of hex
output maybe?), you could use invokeAndWait() instead of invokeLater().
That way, you don't even need to save the byte buffer in a vector because
your GUI code will be called for each buffer you read. Just change the
getData() call to return the current buffer instead.

You might also consider checking out the SwingWorker class which is
designed for taking care of all the overhead of doing just this sort of
task for you.

--
Curt Welch http://CurtWelch.Com/
cu...@kcwc.com http://NewsReader.Com/

0 new messages