Patch for adding SO_REUSEPORT socket option

Alan Bateman Alan.Bateman at oracle.com
Fri Jan 15 12:29:16 UTC 2016


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


More information about the net-dev mailing list