Patch to fix BufferedInputStream.skip and bug 4254082

Bill Pugh pugh at cs.umd.edu
Tue Jul 10 22:50:22 UTC 2007


The proposed change is that BufferedInputStream skip should not skip  
less bytes than requested unless there are no buffered bytes and a  
call to skip on the wrapped input stream returns 0.

Since the return value of skip is rarely checked, a short skip will  
almost certainly result in program failure.

As a consequence of this change, skip may block in situations where  
it could avoid blocking by performing a short skip. Since I believe  
that a short skip is likely to result in program failure, and  
blocking is better than failure, I believe this is an acceptable change.

	Bill


On Jul 10, 2007, at 3:35 PM, Christopher Hegarty - Sun Microsystems  
wrote:

> Hi Pascal, Bill, et al,
>
> I was responsible for integrating the contributed fix for 6192696.  
> The problem with it was that it relied on a 'good' implementation  
> of the available method. By 'good' I mean that it actually returns  
> the amount of data that can be read without blocking, and not  
> simply 1. ZipInputStream.available simply returns 1 (if there is  
> any data available) as it is difficult to determine the amount of  
> available data when dynamically uncompressing a data stream.
>
> Now, using available to try and fill as much of  
> BufferedInputStream's internal buffer without blocking may result  
> in reading only 1 byte at a time from the underlying stream,  
> creating a performance degradation. See
> 6409506 and 6411870.
>
> I'm not sure what exactly is being proposed here, but if available  
> is going to be used to optimize the amount of data actually  
> skipped, be aware of the limitation of ZipInputStream.available and  
> other 'bad' available implementations.
>
> -Chris.
>
> Pascal S. de Kloe wrote:
>> Hello Martin,
>>> 6192696: BufferedInputStream.read(byte[], int, int) can block if the
>>> entire buffer can't be filled
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6192696
>> The problem is fill(). It doesn't check available(). The patch on  
>> this mailing list is supposed to fix that too.




More information about the core-libs-dev mailing list