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