Thread Safety

The LittleEndianInputStream class is not threadsafe. Consider the readInt( ) method:

public int readInt( ) throws IOException { int byte1 = in.read( ); int byte2 = in.read( ); int byte3 = in.read( ); int byte4 = in.read( ); if (byte4 == -1 || byte3 == -1 || byte2 == -1 || byte1 == -1) { throw new EOFException( ); } return (byte4 << 24) + (byte3 << 16) + (byte2 << 8) + byte1; }

If two threads are trying to read from this input stream at the same time, there is no guarantee that bytes 1 through 4 will be read in order. The first thread might read bytes 1 and 2, and then the second thread could preempt it and read any number of bytes. When the first thread regained control, it would no longer be able to read bytes 3 and 4 but would read whichever bytes happened to be next in line. It would then return an erroneous result.

A synchronized block would solve this problem neatly:

public int readInt( ) throws IOException { int byte1, byte2, byte3, byte4; synchronized (this) { byte1 = in.read( ); byte2 = in.read( ); byte3 = in.read( ); byte4 = in.read( ); } if (byte4 == -1 || byte3 == -1 || byte2 == -1 || byte1 == -1) { throw new EOFException( ); } return (byte4 << 24) + (byte3 << 16) + (byte2 << 8) + byte1; }

It isn't necessary to synchronize the entire methodonly the four lines that read from the underlying stream. However, this solution is still imperfect. It is remotely possible that another thread has a reference to the underlying stream rather than to the little-endian input stream and could try to read directly from that. Therefore, you might be better off synchronizing on the underlying input stream in.

However, this would prevent another thread from reading from the underlying input stream only if the second thread also synchronized on the underlying input stream. In general, you can't count on this, so it's not really a solution. In fact, Java really doesn't provide a good means to guarantee thread safety when you have to modify objects you don't control that are passed as arguments to your methods.

LittleEndianOutputStream has equally severe problems. Consider the writeInt( ) method:

public void writeInt(int i) throws IOException { out.write(i & 0xFF); out.write((i >>> 8) & 0xFF); out.write((i >>> 16) & 0xFF); // What happens if another thread preempts here? out.write((i >>> 24) & 0xFF); written += 4; }

Suppose a second thread preempts the running thread where indicated and writes unrelated data onto the output stream. The entire stream can be corrupted because the bytes of the int are separated. All the problems I've noted here are shared by DataInputStream and DataOutputStream, and similar problems crop up in other filter stream classes. This leads to the following general principle for threadsafe programming:

Never allow two threads to share a stream.

The principle is most obvious for filter streams, but it applies to regular streams as well. Although writing or reading a single byte can be treated as an atomic operation, many programs will not be happy to read and write individual bytes. They'll want to read or write a particular group of bytes and will not react well to being interrupted.