[PATCH] socket SOCK_CLOEXEC #15889
Comments
From alexander.bluhm@gmx.netCreated by alexander.bluhm@gmx.netWhen analyzing system calls, I found that Perl socket always does I know that my patch is only a micro optimization, but it should I have startet to implement this for socket(2). If you agree that To get all tests pass, I had to adjust my email in AUTHORS Perl Info |
From alexander.bluhm@gmx.net0001-Use-SOCK_CLOEXEC-when-creating-a-socket.patchFrom 6ea25c1667c047465291ad52a7ff90f00bc97777 Mon Sep 17 00:00:00 2001
From: Alexander Bluhm <bluhm@cpan.org>
Date: Tue, 21 Feb 2017 19:12:29 +0100
Subject: [PATCH] Use SOCK_CLOEXEC when creating a socket
If the system provides the SOCK_CLOEXEC close-on-exec flag for
sockets, use this feature to avoid an unnecessary fnctl(F_SETFD)
system call. In the very uncommon case that a system file descriptor
is created, clear the FD_CLOEXEC flag afterwards.
---
AUTHORS | 2 +-
MANIFEST | 1 +
Porting/checkAUTHORS.pl | 3 +++
pp_sys.c | 10 +++++++++-
t/io/cloexec.t | 43 +++++++++++++++++++++++++++++++++++++++++++
5 files changed, 57 insertions(+), 2 deletions(-)
create mode 100644 t/io/cloexec.t
diff --git a/AUTHORS b/AUTHORS
index 4e4756b494..260ea42073 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -60,7 +60,7 @@ Alex Solovey <a.solovey@gmail.com>
Alex Vandiver <alexmv@mit.edu>
Alex Waugh <alex@alexwaugh.com>
Alexander Alekseev <alex@alemate.ru>
-Alexander Bluhm <alexander_bluhm@genua.de>
+Alexander Bluhm <bluhm@cpan.org>
Alexander D'Archangel <darksuji@gmail.com>
Alexander Gernler <alexander_gernler@genua.de>
Alexander Gough <alex-p5p@earth.li>
diff --git a/MANIFEST b/MANIFEST
index ef1d98472e..605ff2eb91 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -5309,6 +5309,7 @@ t/harness Finer diagnostics from test suite
t/io/argv.t See if ARGV stuff works
t/io/binmode.t See if binmode() works
t/io/bom.t See if scripts can start with a byte order mark
+t/io/cloexec.t See if file descriptor has FD_CLOEXEC flag
t/io/closepid.t See if close works for subprocesses
t/io/crlf.t See if :crlf works
t/io/crlf_through.t See if pipe passes data intact with :crlf
diff --git a/Porting/checkAUTHORS.pl b/Porting/checkAUTHORS.pl
index cb8863c79a..bdd179924b 100755
--- a/Porting/checkAUTHORS.pl
+++ b/Porting/checkAUTHORS.pl
@@ -563,6 +563,9 @@ bert\100alum.mit.edu bert\100genscan.com
bigbang7\100gmail.com ddascalescu+github\100gmail.com
blgl\100stacken.kth.se blgl\100hagernas.com
+ 2bfjdsla52kztwejndzdstsxl9athp\100gmail.com
+bluhm\100cpan.org alexander.bluhm\100gmx.net
++ alexander_bluhm\100genua.de
++ bluhm\100openbsd.org
brian.d.foy\100gmail.com bdfoy\100cpan.org
BQW10602\100nifty.com sadahiro\100cpan.org
bulk88\100hotmail.com bulk88
diff --git a/pp_sys.c b/pp_sys.c
index 7a5703515c..d8c004afd1 100644
--- a/pp_sys.c
+++ b/pp_sys.c
@@ -2512,7 +2512,7 @@ PP(pp_socket)
{
dSP;
const int protocol = POPi;
- const int type = POPi;
+ int type = POPi;
const int domain = POPi;
GV * const gv = MUTABLE_GV(POPs);
IO * const io = GvIOn(gv);
@@ -2521,6 +2521,9 @@ PP(pp_socket)
if (IoIFP(io))
do_close(gv, FALSE);
+#if defined(SOCK_CLOEXEC)
+ type |= SOCK_CLOEXEC;
+#endif
TAINT_PROPER("socket");
fd = PerlSock_socket(domain, type, protocol);
if (fd < 0) {
@@ -2536,10 +2539,15 @@ PP(pp_socket)
RETPUSHUNDEF;
}
#if defined(HAS_FCNTL) && defined(F_SETFD) && defined(FD_CLOEXEC)
+#if defined(SOCK_CLOEXEC)
+ if (fd <= PL_maxsysfd && fcntl(fd, F_SETFD, 0) < 0)
+ RETPUSHUNDEF;
+#else
/* ensure close-on-exec */
if (fd > PL_maxsysfd && fcntl(fd, F_SETFD, FD_CLOEXEC) < 0)
RETPUSHUNDEF;
#endif
+#endif
RETPUSHYES;
}
diff --git a/t/io/cloexec.t b/t/io/cloexec.t
new file mode 100644
index 0000000000..1dc695b50d
--- /dev/null
+++ b/t/io/cloexec.t
@@ -0,0 +1,43 @@
+#!perl
+
+# test wether close-on-exec file descriptor flag is set initially
+
+BEGIN {
+ chdir 't' if -d 't';
+ @INC = '../lib';
+ require "./test.pl";
+ skip_all_if_miniperl();
+ skip_all_without_config('d_fcntl');
+ require Config; import Config;
+ require Fcntl; import Fcntl, qw(F_GETFD FD_CLOEXEC);
+}
+
+use strict;
+
+plan tests => 8;
+
+SKIP: {
+ $Config{d_socket} or skip("No socket", 3);
+ $Config{extensions} =~ /\bSocket\b/ or skip("No Socket", 3);
+ require Socket; import Socket, qw(PF_INET SOCK_STREAM);
+ ok(socket(my $sock, Socket::PF_INET(), Socket::SOCK_STREAM(), 0),
+ "socket", "socket PF_INET SOCK_STREAM failed: $!");
+ my $flags = fcntl($sock, F_GETFD, 0);
+ ok($flags, "fcntl socket", "fcntl F_GETFD failed: $!");
+ is($flags & FD_CLOEXEC, FD_CLOEXEC, "socket FD_CLOEXEC",
+ "FD_CLOEXEC not set: $flags");
+}
+
+SKIP: {
+ $Config{d_socket} or skip("No socket", 5);
+ $Config{extensions} =~ /\bSocket\b/ or skip("No Socket", 5);
+ require Socket; import Socket, qw(PF_INET SOCK_STREAM);
+ ok(close(STDIN), "close STDIN failed: $!");
+ ok(socket(my $sock, Socket::PF_INET(), Socket::SOCK_STREAM(), 0),
+ "socket", "socket PF_INET SOCK_STREAM failed: $!");
+ is($sock->fileno(), 0, "socket file number not zero with closed stdin");
+ my $flags = fcntl($sock, F_GETFD, 0);
+ ok($flags, "fcntl socket", "fcntl F_GETFD failed: $!");
+ is($flags & FD_CLOEXEC, 0, "socket FD_CLOEXEC",
+ "FD_CLOEXEC set: $flags");
+}
--
2.11.1
|
From zefram@fysh.orgAlexander Bluhm wrote:
It is certainly a good idea to use SOCK_CLOEXEC where it is available, Similar comments apply to the use of O_CLOEXEC with open(2), for which -zefram |
|
The RT System itself - Status changed from 'new' to 'open' |
From zefram@fysh.orgThe logic required is approximately: int socket_cloexec(int domain, int type, int protocol) I haven't tested this code, but it's based on similar code that I wrote -zefram |
From @jkeenanOn Fri, 24 Feb 2017 13:12:12 GMT, alexander.bluhm@gmx.net wrote:
Given code freeze, I'm marking this ticket and its patch for consideration post-5.26.0 release. Thank you very much. -- |
From [Unknown Contact. See original ticket]On Fri, 24 Feb 2017 13:12:12 GMT, alexander.bluhm@gmx.net wrote:
Given code freeze, I'm marking this ticket and its patch for consideration post-5.26.0 release. Thank you very much. -- |
From @jkeenanOn Sun, 26 Feb 2017 21:31:55 GMT, jkeenan wrote:
With perl-5.26.0 having been released, this ticket is now up for consideration. Thank you very much. -- |
From zefram@fysh.orgFixed with a comprehensive set of close-on-exec changes in commit -zefram |
Migrated from rt.perl.org#130851 (status was 'open')
Searchable as RT130851$
The text was updated successfully, but these errors were encountered: