Hi Alan,
Here is a new version of the patch:
http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.08/
In this version, based on your comment, we have changed following items:
1. Remove the dependency of DatagramSocketImpl and MulticastSocket with
AbstractPlainSocketImpl.
2. Made addReusePortOption() to be both thread safe and package private.
Please let us know your feedback and comments.
Thanks,
Lucy
-Original Message-
From: Alan Bateman [mailto:alan.bate...@oracle.com]
Sent: Friday, January 15, 2016 4:29 AM
To: Lu, Yingqi
Cc: net-dev@openjdk.java.net; Viswanathan, Sandhya
; Kharbas, Kishor ;
Aundhe, Shirish ; Kaczmarek, Eric
Subject: Re: Patch for adding SO_REUSEPORT socket option
On 14/01/2016 18:28, Lu, Yingqi wrote:
> Hi Alan/Michael/Volker,
>
> Here is the link to the version 7 of the patch.
> http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.07/
>
> Please note: after the code has been uploaded, I found out I forgot to remove
> extra check of ENOPROTOOPT in the net_util_md.c. Since it took us couple days
> to upload the code each time, I think it might be a good idea to mark this as
> an open item for now. I will change it in next version for sure together with
> anything you found in the current version.
>
>
I think this looks quite good but there is still one architectural issue that
doesn't look quite right to me.
The issue is that DatagramSocketImpl and MulticastSocket shouldn't be dependent
on the built-in AbstractPlainSocketImpl. If you develop your own
DatagramSocketImpl implementation then you'll quickly see the issue.
It looks like you have it right in SocketImpl, it's just DatagramSocketImpl and
MulticastSocket where the dependency seems to exist.
One other comment is that I don't think SocketImpl.addAdditionalOptions is
ready to be exposed in the API (you've added it as a protected method as it
will become part of the Java SE API). Clearly there is a need for impls to have
a way to expand the set of socket options but it needs further consideration on
what that API should be. The simplest is to just drop protected and make this a
package-private method.
One other point on addAdditionalOptions is that it mutates the set of socket
options and so is not thread safe. We'll need to come up with a way to do this
in a safe way.
One final comment is on the Unix version of PlainSocketImpl and
PlainDatagramSocketImpl. I'm wondering if these changes are really needed. This
may be something Michael may have suggestions on.
-Alan
