[PATCH] SOCK_CLOEXEC for opening sockets

[Martin Buchholz]

I agree with this approach - it parallels the efforts made with O_CLOEXEC
in past years.

According to
https://www.freebsd.org/cgi/man.cgi?query=socket&sektion=2
SOCK_CLOEXEC is also available on freebsd.

On Tue, Jul 10, 2018 at 1:36 AM, Andrew Luo <
andrewluotechnologies at outlook.com> wrote:

> Hi,
>
>
>
> I want to propose to use SOCK_CLOEXEC when opening sockets in the
> OpenJDK.  I ran into some issues when forking processes (in
> JNI/C/C++/native code) on Linux where OpenJDK had opened a socket (in Java
> code).  The child process ends up inheriting a file descriptor to the same
> socket, which is not ideal in certain circumstances (for example if the
> Java process restarts and tries to open the socket again while the child
> process is still running).  Of course, after forking the child process can
> close all those file descriptors as a workaround, but we use O_CLOEXEC when
> opening files, so I think it would be ideal to use the same for sockets.
>
>
>
> Just some info about the patch:
>
>
>
> 1.       This is only for Linux (I don’t believe SOCK_CLOEXEC exists on
> other platforms, I use a preprocessor guard for SOCK_CLOEXEC)
>
> 2.       I try twice if the first time attempting to open the socket
> fails with EINVAL because it is possible that the OpenJDK was compiled on a
> newer kernel/with newer headers that supports SOCK_CLOEXEC but runs on a
> lower version kernel (not sure if this is supported by the OpenJDK project)
>
>
>
> Patch is attached below.  Let me know if you want me to make some changes.
>
>
>
> (I did not add a unit test – it would probably need to be a functional
> test, one that involves a child process and forking, etc.  Let me know if
> you believe this is necessary to add)
>
>
>
> Thanks,
>
>
>
> -Andrew
>
>
>
> diff -r 95c0644a1c47 src/java.base/unix/native/libnet/PlainSocketImpl.c
>
> --- a/src/java.base/unix/native/libnet/PlainSocketImpl.c Fri Jun 15
> 17:34:01 2018 -0700
>
> +++ b/src/java.base/unix/native/libnet/PlainSocketImpl.c             Tue
> Jul 10 01:30:08 2018 -0700
>
> @@ -104,6 +104,34 @@
>
> }
>
>  /*
>
> + * Opens a socket
>
> + * On systems where supported, uses SOCK_CLOEXEC where possible
>
> + */
>
> +static int openSocket(int domain, int type, int protocol) {
>
> +#if defined(SOCK_CLOEXEC)
>
> +    int typeToUse = type | SOCK_CLOEXEC;
>
> +#else
>
> +    int typeToUse = type;
>
> +#endif
>
> +
>
> +    int socketFileDescriptor = socket(domain, typeToUse, protocol);
>
> +#if defined(SOCK_CLOEXEC)
>
> +    if ((socketFileDescriptor == -1) && (errno = EINVAL)) {
>
> +        // Attempt to open the socket without SOCK_CLOEXEC
>
> +        // May have been compiled on an OS with SOCK_CLOEXEC supported
>
> +        // But runtime system might not have SOCK_CLOEXEC support
>
> +        socketFileDescriptor = socket(domain, type, protocol);
>
> +    }
>
> +#else
>
> +    // Best effort
>
> +    // Return value is intentionally ignored since socket was
> successfully opened anyways
>
> +    fcntl(socketFileDescriptor, F_SETFD, FD_CLOEXEC);
>
> +#endif
>
> +
>
> +    return socketFileDescriptor;
>
> +}
>
> +
>
> +/*
>
>   * The initroto function is called whenever PlainSocketImpl is
>
>   * loaded, to cache field IDs for efficiency. This is called every time
>
>   * the Java class is loaded.
>
> @@ -178,7 +206,8 @@
>
>          return;
>
>      }
>
> -    if ((fd = socket(domain, type, 0)) == -1) {
>
> +
>
> +    if ((fd = openSocket(domain, type, 0)) == -1) {
>
>          /* note: if you run out of fds, you may not be able to load
>
>           * the exception class, and get a NoClassDefFoundError
>
>           * instead.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20180710/8e0c4ed6/attachment.html>