FULL PRODUCT VERSION :
java version "1.4.2_05"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.2_05-b04)
Java HotSpot(TM) Client VM (build 1.4.2_05-b04, mixed mode)
ADDITIONAL OS VERSION INFORMATION :
Windows XP SP1
A DESCRIPTION OF THE PROBLEM :
The method InputStream.read(byte[], int, int) doens't do what is expected, and can hang a thread when the input stream is wrapped into a BufferedInputStream.
BufferedInputStream's fill() method calls read(byte[], int, int) on nested input stream to read up to a max of bytes. However, InputStream's read method doesn't return until "len" bytes are read or end of stream has been reached.
STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Run attached test application.
java InputStreamReadBug -buffered -> hangs
java InputStreamReadBug -> does not hang
EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
InputStream.read() should wait for one byte, but not wait for any more data that is not available. If calling read(buf, 0, 5) and only 2 bytes are avaiable, read() shold return two bytes only.
REPRODUCIBILITY :
This bug can be reproduced always.
---------- BEGIN SOURCE ----------
import java.io.*;
public class InputStreamReadBug extends InputStream {
byte[] data = { 1, 2, 3, 4, 5};
int current = 0;
private void waitForever() {
try {
Object lock = new Object();
synchronized (lock) {
lock.wait();
}
}
catch (InterruptedException ex ){
}
}
/**
* Simulate a socket that has 5 bytes available, reading
* past that will block on read()
*/
public int read() {
if (data.length - current > 0) {
return data[current++];
}
else {
// no data available, we wait
waitForever();
return -1;
}
}
public int available() {
return data.length - current;
}
public static void main(String[] args) throws Exception {
InputStream x = new InputStreamReadBug();
if (args.length == 1 && args[0].equals("-buffered")) {
x = new BufferedInputStream(x);
}
/* there are 5 bytes available to be read from the
* input stream, so we should be able to do this
* WITHOUT BLOCKING
*/
for (int i=0; i<5; i++) {
System.out.println("reading byte: " + x.read());
}
}
}
---------- END SOURCE ----------
CUSTOMER SUBMITTED WORKAROUND :
If you write a class that extends InputStream, overwrite read(buf, off, len). If you don't you run into problems if your stream is wrapped into a BufferedInputStream.
###@###.### 2004-11-09 12:00:00 GMT
Suggested fix contributed by java.net member leouser:
A DESCRIPTION OF THE FIX :
BUGID: 6192696 BufferedInputStream.read(byte[], int, int) can block if
the entire buffer can't be filled.
This appears also to be BUGID: 4479751
FILES AFFECTED: java.io.BufferedInputStream
JDK VERSION: jdk-6-rc-bin-b64-linux-i586-15_dec_2005.bin
Discussion(embeded in test case as well):
/**
* BUGID: 6192696 BufferedInputStream.read(byte[], int, int) can block if
* the entire buffer can't be filled.
* This appears also to be BUGID: 4479751
*
* This Bug is about blocking when you shouldn't. Sounds like something
* that would happen in a sports game, but here it relates to the BufferedInputStream.
* The problem stems from the fill method calling the decorated InputStreams
* read method. This can block. 'fill' does no checking with the available
* method of the InputStream. This in turns can cause this part of the read
* methods contract that it should not block to be violated.
*
* The implementation I have provided to fix this blocking behavior checks
* the available method, ensuring that a read is only as large as what is
* available. This though by itself may be inadequate. Hence after the
* read, we check again the amount available. This process continues
* until the buffer is full or the InputStream indicates that it doesn't
* have any data available. This will ensure the buffer is filled properly
* if the underlying InputStream makes available more data after it has
* been read thouroughly. It also will help emulate the behavior of
* reading until the buffer is full, if it is possible to fill it up. I guess
* the design is: keep reading while you can.
*
* Another problem that may have been lurking is in the read1 method.
* In this method there is an unchecked call to InputStream.read. This
* could result in the same blocking behavior as we've seen in fill. Therefor
* I have enhanced this code to do the read at most with what is available.
*
* ANTI-RATIONALE: this may break clients that are relying upon this bug. :D
* We also are gaining 4-5 lines of code, but this is necessary to ensure
* the more sophisticated behavior is in place.
*
* TESTING STRATEGY:
* Exercise both read methods and use the custom InputStream as
* the source of the data. Since the custom InputStream only will
* return 1 byte at a time this will test the looping behavior in
* the fill method. The fill is indirectly called by testBIS1 and
* testBIS2. These tests do not exercise the read1 methods loop. But
* upon observation, starting Java instance appears to exercise this
* loop. Since this occurs, does successful startup qualify as a test
* for that piece of code?
*
* Files affect: java.io.BufferedInputStream
* BufferedInputStream enhanced with java 6 version:
* jdk-6-rc-bin-b64-linux-i586-15_dec_2005.bin
*
* test ran succesfully on a SUSE 7.3 Linux distribution
*
* Brian Harry
* ###@###.###
* Jan 15, 2006
*/
UNIFIED DIFF:
--- /home/nstuff/java6/jdk1.6.0/java/io/BufferedInputStream.java Thu Dec 15 02:16:35 2005
+++ /home/javarefs/java/io/BufferedInputStream.java Sun Jan 15 17:04:20 2006
@@ -215,9 +215,16 @@
buffer = nbuf;
}
count = pos;
- int n = getInIfOpen().read(buffer, pos, buffer.length - pos);
- if (n > 0)
- count = n + pos;
+ InputStream ins = getInIfOpen();
+ while(ins.available() > 0 && count != buffer.length){ //read until full or nothing available..
+ int available = ins.available();
+ int amount = buffer.length - count;
+ if(amount > available) amount = available;
+ int n = ins.read(buffer, count, amount);
+ if(n > 0)
+ count = n + count;
+ }
+
}
/**
@@ -251,7 +258,19 @@
bytes into the local buffer. In this way buffered streams will
cascade harmlessly. */
if (len >= getBufIfOpen().length && markpos < 0) {
- return getInIfOpen().read(b, off, len);
+ int amount = 0;
+ InputStream ins = getInIfOpen();
+ int spot = off;
+ int tlen = len;
+ while(ins.available() > 0 && amount < len){
+ int available = ins.available();
+ if(tlen > available) tlen = available;
+ int ramount = ins.read(b, spot, tlen);
+ amount += ramount;
+ spot += ramount; //move ahead
+ tlen = len - ramount; //shrink
+ }
+ return amount;
}
fill();
avail = count - pos;
JUnit TESTCASE :
import junit.framework.TestCase;
import junit.textui.TestRunner;
import static java.lang.System.out;
import java.io.*;
/**
* BUGID: 6192696 BufferedInputStream.read(byte[], int, int) can block if
* the entire buffer can't be filled.
* This appears also to be BUGID: 4479751
*
* This Bug is about blocking when you shouldn't. Sounds like something
* that would happen in a sports game, but here it relates to the BufferedInputStream.
* The problem stems from the fill method calling the decorated InputStreams
* read method. This can block. 'fill' does no checking with the available
* method of the InputStream. This in turns can cause this part of the read
* methods contract that it should not block to be violated.
*
* The implementation I have provided to fix this blocking behavior checks
* the available method, ensuring that a read is only as large as what is
* available. This though by itself may be inadequate. Hence after the
* read, we check again the amount available. This process continues
* until the buffer is full or the InputStream indicates that it doesn't
* have any data available. This will ensure the buffer is filled properly
* if the underlying InputStream makes available more data after it has
* been read thouroughly. It also will help emulate the behavior of
* reading until the buffer is full, if it is possible to fill it up. I guess
* the design is: keep reading while you can.
*
* Another problem that may have been lurking is in the read1 method.
* In this method there is an unchecked call to InputStream.read. This
* could result in the same blocking behavior as we've seen in fill. Therefor
* I have enhanced this code to do the read at most with what is available.
*
* ANTI-RATIONALE: this may break clients that are relying upon this bug. :D
* We also are gaining 4-5 lines of code, but this is necessary to ensure
* the more sophisticated behavior is in place.
*
* TESTING STRATEGY:
* Exercise both read methods and use the custom InputStream as
* the source of the data. Since the custom InputStream only will
* return 1 byte at a time this will test the looping behavior in
* the fill method. The fill is indirectly called by testBIS1 and
* testBIS2. These tests do not exercise the read1 methods loop. But
* upon observation, starting Java instance appears to exercise this
* loop. Since this occurs, does successful startup qualify as a test
* for that piece of code?
*
* Files affect: java.io.BufferedInputStream
* BufferedInputStream enhanced with java 6 version:
* jdk-6-rc-bin-b64-linux-i586-15_dec_2005.bin
*
* test ran succesfully on a SUSE 7.3 Linux distribution
*
* Brian Harry
* ###@###.###
* Jan 15, 2006
*/
public class TestBIS extends TestCase{
public TestBIS(String method){
super(method);
}
public static class INS2 extends InputStream{
byte[] data = {0,1,2,3,4,5};
int current = 0;
private void waitForever(){
try{
Object lock = new Object();
synchronized(lock){
lock.wait();
}
}
catch(InterruptedException ex){}
}
public int read(){
if(data.length - current > 0){
return data[current++];
}
else{
waitForever();
return -1;
}
}
public int available(){
if((data.length - current) != 0 ) return 1;
return -1;
}
}
public void testBIS1(){
//this exercises the read functionality...
out.println();
out.println("Exercising the read() method...");
InputStream x = new BufferedInputStream(new INS2(),3);
try{
for(int i = 0; i < 6;i++){
int b = x.read();
assertEquals(b,i);
out.println("reading byte:" + b );
}
}
catch(IOException io){}
}
public void testBIS2(){
//this exercise the read(byte[], int, int) functionality....
out.println();
out.println("Exercising the read(byte[],int,int) method");
InputStream x = new BufferedInputStream(new INS2(),3);
try{
byte[] b = new byte[100];
out.println( "reading 100 bytes");
int amount = x.read(b,0,100);
assertEquals(amount, 6);
out.println( "Amount read was " + amount);
for(int i = 0; i < 6; i ++){
int b2 = b[i];
assertEquals(b2, i);
out.println("byte " + i + " is ok");
}
}
catch(IOException io){}
}
public static void main(String ... args) throws IOException{
TestCase tc1 = new TestBIS("testBIS1");
TestRunner.run(tc1);
TestCase tc2 = new TestBIS("testBIS2");
TestRunner.run(tc2);
}
}
FIX FOR BUG NUMBER:
6192696, 4479751

Comments

EVALUATION
This fix has a negative performance effect on InputStreams that cannot or do not have a good available() implementation, see 6409506, 6411870. For this reason this fix cannot be keep and must be removed from BufferedInputStream (see 6409506).

2006-04-19

EVALUATION
This issue stems from the fact that you can implement InputStream as a
BLOCKING or NON-BLOCKING stream. Brief explanation:
BLOCKING: There are many InputStream subclasses that simply implement
the read() method, as this is the only abstract method. The other read
methods read(byte[]) and read(byte[],int,int) are essentially blocking,
as in they will try to read the amount of data requested and will not
return until it is read or EOF is reached. Even changing
java.io.InputStream will not necessarily prevent developers from
creating blocking inputStreams.
NON-BLOCKING: An InputStream that will return as many bytes as possible
without blocking. Return number may be less than requested.
Now what should BufferedInputStream do! Well currently we will do what
the wrapped InputStream does, but in the case of blocking streams this
can be a problem. For example, if the user requests a single byte,
BufferedInputStream will try to fill the entire buffer before returning.
This could potentially take a long time and make the app appear slow as
it has to wait until the entire buffer is filled before it can proceed,
even though it may only want to process a single byte.
The fix will support both blocking and non-blocking streams. We try to
read as many bytes as available from the wrapped InputStream without
blocking and return them. If the internal buffer is empty and the
wrapped InputStream has nothing available then we will try to fill the
buffer ( and block if that is the implementation of the wrapped
InputStream ), as before. So the only difference from a compatibility
point of view is that we may return fewer bytes than requested, which is
completely acceptable from the spec and good code should always expect this.

EVALUATION
This definitely appears to be a bug. The cause is that the BufferedInputStream.fill() is called by BIS.read() which is only asking for one byte. fill() however will block until the entire buffer (defaultBufferSize = 8192, or explicit size provided during construction) is filled. It does not check the number available bytes in the InputStream with IS.available(). Since all of the read methods are documented to potentially return fewer bytes than requested, they should do so in cases when they would otherwise block.
This is possibly a duplicate of bug 4479751.