[PATCH] SOCK_CLOEXEC for opening sockets

[Andrew Luo]

Do we need to support compiling on a newer kernel with newer headers and then running the compiled binaries on an older platform?  If so, then I think we probably have to use dlsym to call accept4 at runtime rather than compile time...

Thanks,

-Andrew

-----Original Message-----
From: net-dev <net-dev-bounces at openjdk.java.net> On Behalf Of Ivan Gerasimov
Sent: Wednesday, July 25, 2018 10:04 PM
To: Chris Hegarty <chris.hegarty at oracle.com>
Cc: net-dev at openjdk.java.net
Subject: Re: [PATCH] SOCK_CLOEXEC for opening sockets

Hi Chris!

A couple of minor comments.

1)
if (s != -1 || (s == -1 && errno != EINVAL)) {

This can be simplified to

if (s != -1 || errno != EINVAL) {

2)
What about sockets created with accept():  Shouldn't NET_Accept be modified to set O_CLOEXEC as well?
On Linux accept4() can be used to pass SOCK_CLOEXEC flag.

With kind regards,
Ivan

On 7/25/18 5:49 AM, Chris Hegarty wrote:
> Alan,
>
>> On 25 Jul 2018, at 08:24, Alan Bateman <Alan.Bateman at oracle.com> wrote:
>>
>> ...
>>
>> As I said previously, the patch isn't complete so native code calling fork/exec may still have to deal with other file descriptors that are inherited into the child. I don't object to doing this in phases of course but somehow we have managed to get by for 20 years without this being an issue.
> I added the following to the JIRA description to make this clear:
>
> "This JIRA issue, by itself, does not completely resolve the problem 
> of native code calling fork/exec, it may still have to deal with other 
> file descriptors that are inherited by the child. Instead this JIRA 
> issue is targeted at the socket and channels areas only, other areas 
> should be tackled on a phased approach though separate JIRA issues."
>
>> The updates to the various site to use the NET_* functions are fine. However, I think the new functions in net_util_md.c could be cleaner. I think it would be better to fallback to socket/socketpair + fcntl when the initial call fails with EINVAL.
> Agreed. How about this ( trying to reduce the ifdef blocks, and keep 
> them relatively clean ) :
>
> ---
> JNIEXPORT int JNICALL
> NET_Socket(int domain, int type, int protocol) {
>      int s;
> #if defined(SOCK_CLOEXEC)
>      s = socket(domain, type | SOCK_CLOEXEC, protocol);
>      if (s != -1 || (s == -1 && errno != EINVAL)) {
>          return s;
>      }
> #endif
>      s = socket(domain, type, protocol);
>      if (s != -1) {
>          // Best effort - return value is intentionally ignored since the socket
>          // was successfully created.
>          fcntl(s, F_SETFD, FD_CLOEXEC);
>      }
>      return s;
> }
> ---
>
> Updated webrev:
>    http://cr.openjdk.java.net/~chegar/8207335/webrev.01/
>
> -Chris.

--
With kind regards,
Ivan Gerasimov