Skip Menu |
Report information
Id: 130851
Status: open
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: alexander.bluhm [at] gmx.net
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: core
Perl Version: 5.25.11
Fixed In: (no value)

Attachments


Subject: [PATCH] socket SOCK_CLOEXEC
From: alexander.bluhm [...] gmx.net
To: perlbug [...] perl.org
CC: alexander.bluhm [...] gmx.net
Date: Thu, 23 Feb 2017 17:37:07 +0100 (CET)
Download (untitled) / with headers
text/plain 3.4k
This is a bug report for perl from alexander.bluhm@gmx.net, generated with the help of perlbug 1.40 running under perl 5.25.11. ----------------------------------------------------------------- [Please describe your issue here] When analyzing system calls, I found that Perl socket always does an additional fcntl(F_SETFD,FD_CLOEXEC). Modern socket(2) syscalls support the SOCK_CLOEXEC flag to do this atomically. In general this avoids a threading race, I did it to save the additional syscall. I had to add logic for socket descriptor 0, 1, 2, but this case should not happen in practice. I know that my patch is only a micro optimization, but it should not do any harm. I have startet to implement this for socket(2). If you agree that this is useful, I will also do it for similar operations. I have started a new test file t/io/cloexec.t so I can add other cases later. To get all tests pass, I had to adjust my email in AUTHORS [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=low Type=Patch PatchStatus=HasPatch --- Site configuration information for perl 5.25.11: Configured by bluhm at Thu Feb 23 16:17:51 CET 2017. Summary of my perl5 (revision 5 version 25 subversion 11) configuration: Commit id: 6ea25c1667c047465291ad52a7ff90f00bc97777 Platform: osname=openbsd osvers=6.0 archname=OpenBSD.amd64-openbsd uname='openbsd t430s.bluhm.invalid 6.0 generic.mp#2 amd64 ' config_args='-de -Dusedevel' hint=previous useposix=true d_sigaction=define useithreads=undef usemultiplicity=undef use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n bincompat5005=undef Compiler: cc='cc' ccflags ='-fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_FORTIFY_SOURCE=2' optimize='-O2' cppflags='-fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='' gccversion='4.2.1 20070719 ' gccosandvers='openbsd6.0' intsize=4 longsize=8 ptrsize=8 doublesize=8 byteorder=12345678 doublekind=3 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=3 ivtype='long' ivsize=8 nvtype='double' nvsize=8 Off_t='off_t' lseeksize=8 alignbytes=8 prototype=define Linker and Libraries: ld='cc' ldflags ='-Wl,-E -fstack-protector-strong -L/usr/local/lib' libpth=/usr/lib /usr/local/lib /usr/lib libs=-lpthread -lgdbm -lm -lutil -lc perllibs=-lpthread -lm -lutil -lc libc=/usr/lib/libc.a so=so useshrplib=false libperl=libperl.a gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags=' ' cccdlflags='-DPIC -fPIC ' lddlflags='-shared -fPIC -L/usr/local/lib -fstack-protector-strong' --- @INC for perl 5.25.11: lib /home/bluhm/localperl/lib/site_perl/5.25.10/OpenBSD.amd64-openbsd /home/bluhm/localperl/lib/site_perl/5.25.10 /home/bluhm/localperl/lib/5.25.10/OpenBSD.amd64-openbsd /home/bluhm/localperl/lib/5.25.10 --- Environment for perl 5.25.11: HOME=/home/bluhm LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/bluhm/bin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/X11R6/bin:/usr/local/bin:/usr/local/sbin:/usr/games:/usr/ports/infrastructure/bin PERL_BADLANG (unset) SHELL=/bin/ksh

Message body is not shown because sender requested not to inline it.

Date: Fri, 24 Feb 2017 22:23:37 +0000
To: perl5-porters [...] perl.org
Subject: Re: [perl #130851] [PATCH] socket SOCK_CLOEXEC
From: Zefram <zefram [...] fysh.org>
Alexander Bluhm wrote: Show quoted text
>When analyzing system calls, I found that Perl socket always does >an additional fcntl(F_SETFD,FD_CLOEXEC). Modern socket(2) syscalls >support the SOCK_CLOEXEC flag to do this atomically.
It is certainly a good idea to use SOCK_CLOEXEC where it is available, but more logic is required than your patch supplies. Having the SOCK_CLOEXEC macro defined at compile time does not guarantee that the flag will be supported by the kernel at runtime. If it's not supported, then socket(2) may either yield EINVAL or possibly ignore the flag, returning a non-CLOEXEC fd. It is necessary to check for these outcomes at runtime, checking for the latter by an fcntl(2) F_GETFD, and fall back to the non-SOCK_CLOEXEC behaviour if it's not supported. The check need only be done on the first socket(2) call: if SOCK_CLOEXEC worked once then one can assume that it'll work for subsequent calls in the same run. Similar comments apply to the use of O_CLOEXEC with open(2), for which a corresponding patch would be welcome. -zefram
Date: Sat, 25 Feb 2017 03:36:18 +0000
To: perl5-porters [...] perl.org
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #130851] [PATCH] socket SOCK_CLOEXEC
Download (untitled) / with headers
text/plain 1.1k
The logic required is approximately: int socket_cloexec(int domain, int type, int protocol) { int sock; #ifdef SOCK_CLOEXEC static int strategy = 0; switch (strategy) { case 0: sock = socket(domain, type | SOCK_CLOEXEC, protocol); if (sock != -1) { int fdflags = fcntl(sock, F_GETFD); if (fdflags != -1 && (fdflags & FD_CLOEXEC)) { strategy = 1; } else { strategy = 2; fcntl(sock, F_SETFD, FD_CLOEXEC); } } else if (errno == EINVAL) { sock = socket(domain, type, protocol); if (sock != -1) { strategy = 2; fcntl(sock, F_SETFD, FD_CLOEXEC); } else if (errno != EINVAL) { strategy = 2; } } return sock; case 1: return socket(domain, type | SOCK_CLOEXEC, protocol); } #endif sock = socket(domain, type, protocol); if (sock != -1) fcntl(sock, F_SETFD, FD_CLOEXEC); return sock; } I haven't tested this code, but it's based on similar code that I wrote for other syscalls in Hash::SharedMem. I recommend separating the code out in this manner, into a socket_cloexec() function, even if there's only one caller. -zefram
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.1k
On Fri, 24 Feb 2017 13:12:12 GMT, alexander.bluhm@gmx.net wrote: Show quoted text
> This is a bug report for perl from alexander.bluhm@gmx.net, > generated with the help of perlbug 1.40 running under perl 5.25.11. > > > ----------------------------------------------------------------- > [Please describe your issue here] > > When analyzing system calls, I found that Perl socket always does > an additional fcntl(F_SETFD,FD_CLOEXEC). Modern socket(2) syscalls > support the SOCK_CLOEXEC flag to do this atomically. In general > this avoids a threading race, I did it to save the additional > syscall. I had to add logic for socket descriptor 0, 1, 2, but > this case should not happen in practice. > > I know that my patch is only a micro optimization, but it should > not do any harm. > > I have startet to implement this for socket(2). If you agree that > this is useful, I will also do it for similar operations. I have > started a new test file t/io/cloexec.t so I can add other cases > later. > > To get all tests pass, I had to adjust my email in AUTHORS >
Given code freeze, I'm marking this ticket and its patch for consideration post-5.26.0 release. Thank you very much. -- James E Keenan (jkeenan@cpan.org)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.3k
On Sun, 26 Feb 2017 21:31:55 GMT, jkeenan wrote: Show quoted text
> On Fri, 24 Feb 2017 13:12:12 GMT, alexander.bluhm@gmx.net wrote:
> > This is a bug report for perl from alexander.bluhm@gmx.net, > > generated with the help of perlbug 1.40 running under perl 5.25.11. > > > > > > ----------------------------------------------------------------- > > [Please describe your issue here] > > > > When analyzing system calls, I found that Perl socket always does > > an additional fcntl(F_SETFD,FD_CLOEXEC). Modern socket(2) syscalls > > support the SOCK_CLOEXEC flag to do this atomically. In general > > this avoids a threading race, I did it to save the additional > > syscall. I had to add logic for socket descriptor 0, 1, 2, but > > this case should not happen in practice. > > > > I know that my patch is only a micro optimization, but it should > > not do any harm. > > > > I have startet to implement this for socket(2). If you agree that > > this is useful, I will also do it for similar operations. I have > > started a new test file t/io/cloexec.t so I can add other cases > > later. > > > > To get all tests pass, I had to adjust my email in AUTHORS > >
> > Given code freeze, I'm marking this ticket and its patch for > consideration post-5.26.0 release. >
With perl-5.26.0 having been released, this ticket is now up for consideration. Thank you very much. -- James E Keenan (jkeenan@cpan.org)
Date: Fri, 22 Dec 2017 17:00:31 +0000
To: perl5-porters [...] perl.org
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #130851] [PATCH] socket SOCK_CLOEXEC
Download (untitled) / with headers
text/plain 117b
Fixed with a comprehensive set of close-on-exec changes in commit 842c2139ee2bb06de824c15923ef1aca9b9c504c. -zefram


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org