RFR: 8189366: SocketInputStream.available() should check for eof

[Chris Hegarty]

Daniel,

On 12/10/18 11:26, Daniel Fuchs wrote:
> Hi,
> 
> Maybe a more conservative implementation could have been:
> 
>     int available = impl.available();
>     return eof ? 0 : available;

That buys us little more than we had prior to this change,
since impl.available will still call into native before
checking the EOF status.

If we want to keep this, then we need:

     public int available() throws IOException {
         if (impl.isClosedOrPending()) {
             throw new SocketException("Socket closed");
         }

         if (eof) {
             return 0;
         } else {
             return impl.available();
         }
     }

> I almost suggested that yesterday, but I saw that
> read() already had a logic similar to what Vyom was
> proposing for available:
> 
>   146         // EOF already encountered
>   147         if (eof) {
>   148             return -1;
>   149         }
> 
> which AFAICT  means that read returns -1 instead of throwing
> if the socket is closed and EOF was previously reached.

That is correct. While not intuitive, I don't propose
that we change this. ( if this were a new implementation
then I think it should throw IOE for this scenario, but
we are where we are ).

-Chris.


> best regards,
> 
> -- daniel
> 
> On 12/10/2018 09:55, Chris Hegarty wrote:
>>
>> On 12/10/18 08:29, Alan Bateman wrote:
>>> On 11/10/2018 09:03, vyom tewari wrote:
>>>> Hi Chris,
>>>>
>>>> Thanks for review, please find the updated 
>>>> webrev(http://cr.openjdk.java.net/~vtewari/8189366/webrev0.1/index.html) 
>>>> where i included the test.
>>> Can you explain the behavior change for the closed socket case? Will 
>>> this change mean that available returns 0 when it previously throw 
>>> IOException?
>>
>> You are correct. This is not intentional, or desirable.
>>
>> We should revert the change or add an additional check
>> for isClosedOrPending. Since this is already pushed, I
>> filed a new JIRA issue to track this.
>>
>>    https://bugs.openjdk.java.net/browse/JDK-8212114
>>
>> -Chris.
>>
>