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

2018-10-17 Thread vyom tewari
sure, i will send the webrev  soon. Vyom On Wednesday 17 October 2018 04:31 PM, Chris Hegarty wrote: On 12/10/18 12:58, Daniel Fuchs wrote: ... int available = impl.available(); return eof ? 0 : available; addresses the issue of available potentially returning garbage after EOF wh

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

2018-10-17 Thread Chris Hegarty
On 12/10/18 12:58, Daniel Fuchs wrote: ...     int available = impl.available();     return eof ? 0 : available; addresses the issue of available potentially returning garbage after EOF while being much less risky... Agreed. The above resolves the original reported potential bug, without c

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

2018-10-12 Thread Alan Bateman
On 12/10/2018 14:43, David Lloyd wrote: On Fri, Oct 12, 2018 at 6:01 AM Chris Hegarty wrote: 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 ). I am glad t

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

2018-10-12 Thread David Lloyd
On Fri, Oct 12, 2018 at 6:01 AM Chris Hegarty wrote: > 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 ). I am glad then that it is not a new implementa

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

2018-10-12 Thread Daniel Fuchs
On 12/10/2018 12:01, Chris Hegarty wrote: 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.isClose

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

2018-10-12 Thread vyom tewari
On Friday 12 October 2018 04:31 PM, Chris Hegarty wrote: 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 c

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

2018-10-12 Thread 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 befor

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

2018-10-12 Thread Daniel Fuchs
Hi, Maybe a more conservative implementation could have been: int available = impl.available(); return eof ? 0 : 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 encounter

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

2018-10-12 Thread Chris Hegarty
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 c

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

2018-10-12 Thread Alan Bateman
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

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

2018-10-11 Thread Chris Hegarty
> On 11 Oct 2018, at 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. Thanks. I think this is good. -Chris.

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

2018-10-11 Thread vyom tewari
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. Thanks, Vyom On Wednesday 10 October 2018 09:50 PM, Chris Hegarty wrote: Vyom, On 10/10/18 14:16, vyom tewari wrote: Hi All, Please p

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

2018-10-10 Thread Chris Hegarty
Vyom, On 10/10/18 14:16, vyom tewari wrote: Hi All, Please provide your review comments on below fix. Thanks, Vyom On Friday 27 October 2017 11:31 AM, vyom tewari wrote: Wow, is it really almost a year since this was first discussed! ... http://cr.openjdk.java.net/~vtewari/8189366/webr

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

2018-10-10 Thread vyom tewari
Hi All, Please provide your review comments on below fix. Thanks, Vyom On Friday 27 October 2017 11:31 AM, vyom tewari wrote: On Thursday 26 October 2017 03:14 PM, Bernd Eckenfels wrote: What is currently returned at the end of a stream? This looks like a dangerous thing to do, if a exis

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

2017-10-26 Thread vyom tewari
On Thursday 26 October 2017 03:14 PM, Bernd Eckenfels wrote: What is currently returned at the end of a stream? This looks like a dangerous thing to do, if a existing implementation only Currently it returns 0 at end of stream and  same as after change. As David pointed out that ultimately it

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

2017-10-26 Thread David Lloyd
On Thu, Oct 26, 2017 at 4:44 AM, Bernd Eckenfels wrote: > What is currently returned at the end of a stream? This looks like a > dangerous thing to do, if a existing implementation only read when something > is available it might never detect that it reached EOF. At present it ultimately delegate

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

2017-10-26 Thread Martin Buchholz
Any change that fiddles with "available" is never simple! I confess to not understanding sockets, but intuitively they differ from files in that eof is a murky concept - there may not be any data from the other end of the socket right now, but who knows what's coming soon? What's the difference be

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

2017-10-26 Thread Bernd Eckenfels
What is currently returned at the end of a stream? This looks like a dangerous thing to do, if a existing implementation only read when something is available it might never detect that it reached EOF. Gruss Bernd -- http://bernd.eckenfels.net From: net-dev on b