RFR JDK-8024832: (so) ServerSocketChannel.socket().accept() throws IllegalBlockingModeException if not bound
Hi everyone, Could you please review my change for JDK-8024832? http://cr.openjdk.java.net/~prappo/8024832/webrev.00/ Thanks -Pavel
Re: RFR JDK-8024832: (so) ServerSocketChannel.socket().accept() throws IllegalBlockingModeException if not bound
On 27/05/14 10:40, Pavel Rappo wrote: Hi everyone, Could you please review my change for JDK-8024832? http://cr.openjdk.java.net/~prappo/8024832/webrev.00/ Thanks -Pavel Just wondering, what does ServerSocket.accept() throw in the same situation? Michael
Re: RFR JDK-8024832: (so) ServerSocketChannel.socket().accept() throws IllegalBlockingModeException if not bound
Looks good to me Pavel. I can sponsor this change for you. -Chris. On 27 May 2014, at 10:40, Pavel Rappo wrote: > Hi everyone, > > Could you please review my change for JDK-8024832? > > http://cr.openjdk.java.net/~prappo/8024832/webrev.00/ > > Thanks > -Pavel
Re: RFR JDK-8024832: (so) ServerSocketChannel.socket().accept() throws IllegalBlockingModeException if not bound
I think it should be throwing a SocketException rather than a NotYetBoundException to be consistent with ServerSocket.accept() Michael. On 27/05/14 10:56, Chris Hegarty wrote: Looks good to me Pavel. I can sponsor this change for you. -Chris. On 27 May 2014, at 10:40, Pavel Rappo wrote: Hi everyone, Could you please review my change for JDK-8024832? http://cr.openjdk.java.net/~prappo/8024832/webrev.00/ Thanks -Pavel
Re: RFR JDK-8024832: (so) ServerSocketChannel.socket().accept() throws IllegalBlockingModeException if not bound
On 27/05/2014 10:40, Pavel Rappo wrote: Hi everyone, Could you please review my change for JDK-8024832? http://cr.openjdk.java.net/~prappo/8024832/webrev.00/ Can you try just removing the isBound check? I ask because ssc.accept() will throw NotYetBoundException if the ServerSocketChannel is not bound and that should get mapped to the SocketException to match ServerSocket behavior. -Alan.
Re: RFR JDK-8024832: (so) ServerSocketChannel.socket().accept() throws IllegalBlockingModeException if not bound
Michael,
java.net.ServerSocket.accept:
if (!isBound())
throw new SocketException("Socket is not bound yet");
sun.nio.ch.ServerSocketAdaptor.accept will translate
java.nio.channels.NotYetBoundException into an instance of SocketException with
the text you see above. Basically that's what the attached test verifies.
-Pavel
On 27 May 2014, at 10:49, Michael McMahon wrote:
> On 27/05/14 10:40, Pavel Rappo wrote:
>> Hi everyone,
>>
>> Could you please review my change for JDK-8024832?
>>
>> http://cr.openjdk.java.net/~prappo/8024832/webrev.00/
>>
>> Thanks
>> -Pavel
>
> Just wondering, what does ServerSocket.accept() throw in the same situation?
>
> Michael
Re: RFR JDK-8024832: (so) ServerSocketChannel.socket().accept() throws IllegalBlockingModeException if not bound
Okay, that's fine then. I didn't see the call to the translation method
in the diff.
Thanks,
Michael.
On 27/05/14 11:05, Pavel Rappo wrote:
Michael,
java.net.ServerSocket.accept:
if (!isBound())
throw new SocketException("Socket is not bound yet");
sun.nio.ch.ServerSocketAdaptor.accept will translate
java.nio.channels.NotYetBoundException into an instance of SocketException with
the text you see above. Basically that's what the attached test verifies.
-Pavel
On 27 May 2014, at 10:49, Michael McMahon wrote:
On 27/05/14 10:40, Pavel Rappo wrote:
Hi everyone,
Could you please review my change for JDK-8024832?
http://cr.openjdk.java.net/~prappo/8024832/webrev.00/
Thanks
-Pavel
Just wondering, what does ServerSocket.accept() throw in the same situation?
Michael
Re: RFR JDK-8024832: (so) ServerSocketChannel.socket().accept() throws IllegalBlockingModeException if not bound
Alan,
I don't think it would be exactly the same behaviour since there's a
conditional check (1) before the ssc.accept call:
try {
if (!ssc.isBound())
throw new NotYetBoundException();
(1) if (timeout == 0) {
SocketChannel sc = ssc.accept();
if (sc == null && !ssc.isBlocking())
throw new IllegalBlockingModeException();
return sc.socket();
}
(2)ssc.configureBlocking(false);
try {
SocketChannel sc;
(3)if ((sc = ssc.accept()) != null)
So if I remove this explicit check, than depending on condition (1) it can be
the case that (3) will be executed after (2). And (2) can throw a bunch of
different exceptions, hiding the otherwise initial NotYetBoundException.
If it's not a concern I'm happy to remove this check.
-Pavel
On 27 May 2014, at 11:03, Alan Bateman wrote:
> On 27/05/2014 10:40, Pavel Rappo wrote:
>> Hi everyone,
>>
>> Could you please review my change for JDK-8024832?
>>
>> http://cr.openjdk.java.net/~prappo/8024832/webrev.00/
>>
> Can you try just removing the isBound check? I ask because ssc.accept() will
> throw NotYetBoundException if the ServerSocketChannel is not bound and that
> should get mapped to the SocketException to match ServerSocket behavior.
>
> -Alan.
Re: RFR JDK-8024832: (so) ServerSocketChannel.socket().accept() throws IllegalBlockingModeException if not bound
On 27/05/2014 11:19, Pavel Rappo wrote:
Alan,
I don't think it would be exactly the same behaviour since there's a
conditional check (1) before the ssc.accept call:
try {
if (!ssc.isBound())
throw new NotYetBoundException();
(1) if (timeout == 0) {
SocketChannel sc = ssc.accept();
if (sc == null && !ssc.isBlocking())
throw new IllegalBlockingModeException();
return sc.socket();
}
(2)ssc.configureBlocking(false);
try {
SocketChannel sc;
(3)if ((sc = ssc.accept()) != null)
So if I remove this explicit check, than depending on condition (1) it can be
the case that (3) will be executed after (2). And (2) can throw a bunch of
different exceptions, hiding the otherwise initial NotYetBoundException.
If it's not a concern I'm happy to remove this check.
For the no-timeout case then it would be the same. For the timeout case
it it just means that the channel might be changed to non-blocking
before the exception is thrown. I don't think this is a problem as it
will be restored anyway. What you have is fine, I think just removing is
fine too.
-Alan.
Re: RFR JDK-8024832: (so) ServerSocketChannel.socket().accept() throws IllegalBlockingModeException if not bound
2014/5/26 20:01 -0700, michael.x.mcma...@oracle.com: > I think it should be throwing a SocketException rather than a > NotYetBoundException to be consistent with ServerSocket.accept() No, NotYetBoundException is the correct exception here. The NIO exception hierarchy was intentionally designed to be richer than just "SocketException". Consistency with the exceptions thrown by the original java.net APIs is an anti-goal. - Mark
Re: RFR JDK-8024832: (so) ServerSocketChannel.socket().accept() throws IllegalBlockingModeException if not bound
2014/5/27 1:10 -0700, michael.x.mcma...@oracle.com: > On 27/05/14 15:56, mark.reinh...@oracle.com wrote: >> 2014/5/26 20:01 -0700, michael.x.mcma...@oracle.com: >>> I think it should be throwing a SocketException rather than a >>> NotYetBoundException to be consistent with ServerSocket.accept() >> No, NotYetBoundException is the correct exception here. The NIO >> exception hierarchy was intentionally designed to be richer than >> just "SocketException". Consistency with the exceptions thrown >> by the original java.net APIs is an anti-goal. > > The point was clarified on a separate conversation on net-dev. > The method in question refers to the channel's server socket adapter > (ie an instance of java.net.ServerSocket) rather than the channel itself. Ah, okay then -- I didn't realize that from the available context. - Mark
Re: RFR JDK-8024832: (so) ServerSocketChannel.socket().accept() throws IllegalBlockingModeException if not bound
On 27/05/14 15:56, mark.reinh...@oracle.com wrote: 2014/5/26 20:01 -0700, michael.x.mcma...@oracle.com: I think it should be throwing a SocketException rather than a NotYetBoundException to be consistent with ServerSocket.accept() No, NotYetBoundException is the correct exception here. The NIO exception hierarchy was intentionally designed to be richer than just "SocketException". Consistency with the exceptions thrown by the original java.net APIs is an anti-goal. - Mark Hi Mark, The point was clarified on a separate conversation on net-dev. The method in question refers to the channel's server socket adapter (ie an instance of java.net.ServerSocket) rather than the channel itself. What I overlooked was that the exception is being caught correctly and converted to the right exception expected by ServerSocket. Michael.
