New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[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: