* [PATCH 1/2] tests: common tcp_server and unix_server helpers
2019-06-30 22:19 [PATCH 0/2] warn on inheriting blocking sockets Eric Wong
@ 2019-06-30 22:19 ` Eric Wong
2019-06-30 22:19 ` [PATCH 2/2] daemon: warn on inheriting blocking listeners Eric Wong
1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2019-06-30 22:19 UTC (permalink / raw)
To: meta
IO::Socket:*->new options are verbose and we can save
a bunch of code by putting this into t/common.perl,
since the related spawn_listener stuff is already there.
---
t/common.perl | 18 ++++++++++++++++++
t/git-http-backend.t | 9 +--------
t/httpd-corner.t | 15 ++-------------
t/httpd-https.t | 9 +--------
t/httpd.t | 10 +---------
t/nntpd-tls.t | 11 ++---------
t/nntpd.t | 10 ++--------
t/perf-nntpd.t | 9 +--------
t/v2mirror.t | 8 +-------
t/v2writable.t | 9 +--------
t/www_listing.t | 9 +--------
11 files changed, 31 insertions(+), 86 deletions(-)
diff --git a/t/common.perl b/t/common.perl
index 5a898e32..3f05b68a 100644
--- a/t/common.perl
+++ b/t/common.perl
@@ -17,6 +17,24 @@ sub stream_to_string {
$str;
}
+sub tcp_server () {
+ IO::Socket::INET->new(
+ LocalAddr => '127.0.0.1',
+ ReuseAddr => 1,
+ Proto => 'tcp',
+ Type => Socket::SOCK_STREAM(),
+ Listen => 1024,
+ )
+}
+
+sub unix_server ($) {
+ IO::Socket::UNIX->new(
+ Listen => 1024,
+ Type => Socket::SOCK_STREAM(),
+ Local => $_[0],
+ )
+}
+
sub spawn_listener {
my ($env, $cmd, $socks) = @_;
my $pid = fork;
diff --git a/t/git-http-backend.t b/t/git-http-backend.t
index fc2d5462..946cd86a 100644
--- a/t/git-http-backend.t
+++ b/t/git-http-backend.t
@@ -24,14 +24,7 @@ my $tmpdir = tempdir('pi-git-http-backend-XXXXXX', TMPDIR => 1, CLEANUP => 1);
my $err = "$tmpdir/stderr.log";
my $out = "$tmpdir/stdout.log";
my $httpd = 'blib/script/public-inbox-httpd';
-my %opts = (
- LocalAddr => '127.0.0.1',
- ReuseAddr => 1,
- Proto => 'tcp',
- Type => SOCK_STREAM,
- Listen => 1024,
-);
-my $sock = IO::Socket::INET->new(%opts);
+my $sock = tcp_server();
my $host = $sock->sockhost;
my $port = $sock->sockport;
my $pid;
diff --git a/t/httpd-corner.t b/t/httpd-corner.t
index 1cfc2565..5efb9d14 100644
--- a/t/httpd-corner.t
+++ b/t/httpd-corner.t
@@ -28,14 +28,7 @@ my $err = "$tmpdir/stderr.log";
my $out = "$tmpdir/stdout.log";
my $httpd = 'blib/script/public-inbox-httpd';
my $psgi = "./t/httpd-corner.psgi";
-my %opts = (
- LocalAddr => '127.0.0.1',
- ReuseAddr => 1,
- Proto => 'tcp',
- Type => SOCK_STREAM,
- Listen => 1024,
-);
-my $sock = IO::Socket::INET->new(%opts);
+my $sock = tcp_server();
# Make sure we don't clobber socket options set by systemd or similar
# using socket activation:
@@ -56,11 +49,7 @@ if ($^O eq 'linux') {
}
my $upath = "$tmpdir/s";
-my $unix = IO::Socket::UNIX->new(
- Listen => 1024,
- Type => SOCK_STREAM,
- Local => $upath
-);
+my $unix = unix_server($upath);
ok($unix, 'UNIX socket created');
my $pid;
END { kill 'TERM', $pid if defined $pid };
diff --git a/t/httpd-https.t b/t/httpd-https.t
index f6b9806a..93966949 100644
--- a/t/httpd-https.t
+++ b/t/httpd-https.t
@@ -24,14 +24,7 @@ my $tmpdir = tempdir('pi-httpd-https-XXXXXX', TMPDIR => 1, CLEANUP => 1);
my $err = "$tmpdir/stderr.log";
my $out = "$tmpdir/stdout.log";
my $httpd = 'blib/script/public-inbox-httpd';
-my %opts = (
- LocalAddr => '127.0.0.1',
- ReuseAddr => 1,
- Proto => 'tcp',
- Type => SOCK_STREAM,
- Listen => 1024,
-);
-my $https = IO::Socket::INET->new(%opts);
+my $https = tcp_server();
my ($pid, $tail_pid);
END {
foreach ($pid, $tail_pid) {
diff --git a/t/httpd.t b/t/httpd.t
index e085c4b9..e0a2bf44 100644
--- a/t/httpd.t
+++ b/t/httpd.t
@@ -24,15 +24,7 @@ my $addr = $group . '@example.com';
my $cfgpfx = "publicinbox.$group";
my $httpd = 'blib/script/public-inbox-httpd';
my $init = 'blib/script/public-inbox-init';
-
-my %opts = (
- LocalAddr => '127.0.0.1',
- ReuseAddr => 1,
- Proto => 'tcp',
- Type => SOCK_STREAM,
- Listen => 1024,
-);
-my $sock = IO::Socket::INET->new(%opts);
+my $sock = tcp_server();
my $pid;
use_ok 'PublicInbox::Git';
use_ok 'PublicInbox::Import';
diff --git a/t/nntpd-tls.t b/t/nntpd-tls.t
index e3ecdd4f..82b63f3e 100644
--- a/t/nntpd-tls.t
+++ b/t/nntpd-tls.t
@@ -37,15 +37,8 @@ my $pi_config = "$tmpdir/pi_config";
my $group = 'test-nntpd-tls';
my $addr = $group . '@example.com';
my $nntpd = 'blib/script/public-inbox-nntpd';
-my %opts = (
- LocalAddr => '127.0.0.1',
- ReuseAddr => 1,
- Proto => 'tcp',
- Type => SOCK_STREAM,
- Listen => 1024,
-);
-my $starttls = IO::Socket::INET->new(%opts);
-my $nntps = IO::Socket::INET->new(%opts);
+my $starttls = tcp_server();
+my $nntps = tcp_server();
my ($pid, $tail_pid);
END {
foreach ($pid, $tail_pid) {
diff --git a/t/nntpd.t b/t/nntpd.t
index bf5bb883..0e59de07 100644
--- a/t/nntpd.t
+++ b/t/nntpd.t
@@ -36,14 +36,8 @@ SKIP: {
use_ok 'PublicInbox::V2Writable';
}
-my %opts = (
- LocalAddr => '127.0.0.1',
- ReuseAddr => 1,
- Proto => 'tcp',
- Type => SOCK_STREAM,
- Listen => 1024,
-);
-my $sock = IO::Socket::INET->new(%opts);
+my %opts;
+my $sock = tcp_server();
my $pid;
my $len;
END { kill 'TERM', $pid if defined $pid };
diff --git a/t/perf-nntpd.t b/t/perf-nntpd.t
index 3ca1ce33..ab11b037 100644
--- a/t/perf-nntpd.t
+++ b/t/perf-nntpd.t
@@ -44,14 +44,7 @@ if (($ENV{NNTP_TEST_URL} || '') =~ m!\Anntp://([^/]+)/([^/]+)\z!) {
close $fh or die "close($pi_config): $!";
}
- %opts = (
- LocalAddr => '127.0.0.1',
- ReuseAddr => 1,
- Proto => 'tcp',
- Listen => 1024,
- );
- my $sock = IO::Socket::INET->new(%opts);
-
+ my $sock = tcp_server();
ok($sock, 'sock created');
my $cmd = [ $nntpd, '-W0' ];
$pid = spawn_listener({ PI_CONFIG => $pi_config }, $cmd, [$sock]);
diff --git a/t/v2mirror.t b/t/v2mirror.t
index c31dcd5b..9bc32f30 100644
--- a/t/v2mirror.t
+++ b/t/v2mirror.t
@@ -60,17 +60,11 @@ my $epoch_max = $v2w->{epoch_max};
ok($epoch_max > 0, "multiple epochs");
$v2w->done;
-my %opts = (
- LocalAddr => '127.0.0.1',
- ReuseAddr => 1,
- Proto => 'tcp',
- Listen => 1024,
-);
my ($sock, $pid);
END { kill 'TERM', $pid if defined $pid };
$! = 0;
-$sock = IO::Socket::INET->new(%opts);
+$sock = tcp_server();
ok($sock, 'sock created');
my $cmd = [ "$script-httpd", "--stdout=$tmpdir/out", "--stderr=$tmpdir/err" ];
ok(defined($pid = spawn_listener(undef, $cmd, [ $sock ])),
diff --git a/t/v2writable.t b/t/v2writable.t
index 8f32fbe5..5406fd1b 100644
--- a/t/v2writable.t
+++ b/t/v2writable.t
@@ -134,13 +134,6 @@ if ('ensure git configs are correct') {
use IO::Socket::INET;
my $err = "$mainrepo/stderr.log";
my $out = "$mainrepo/stdout.log";
- my %opts = (
- LocalAddr => '127.0.0.1',
- ReuseAddr => 1,
- Proto => 'tcp',
- Type => SOCK_STREAM,
- Listen => 1024,
- );
my $group = 'inbox.comp.test.v2writable';
my $pi_config = "$mainrepo/pi_config";
open my $fh, '>', $pi_config or die "open: $!\n";
@@ -153,7 +146,7 @@ if ('ensure git configs are correct') {
EOF
;
close $fh or die "close: $!\n";
- my $sock = IO::Socket::INET->new(%opts);
+ my $sock = tcp_server();
ok($sock, 'sock created');
my $pid;
my $len;
diff --git a/t/www_listing.t b/t/www_listing.t
index e5b797db..a5d81f38 100644
--- a/t/www_listing.t
+++ b/t/www_listing.t
@@ -73,14 +73,7 @@ SKIP: {
my $v2 = "$tmpdir/v2";
my $httpd = 'blib/script/public-inbox-httpd';
use IO::Socket::INET;
- my %opts = (
- LocalAddr => '127.0.0.1',
- ReuseAddr => 1,
- Proto => 'tcp',
- Type => SOCK_STREAM,
- Listen => 1024,
- );
- my $sock = IO::Socket::INET->new(%opts);
+ my $sock = tcp_server();
ok($sock, 'sock created');
my ($host, $port) = ($sock->sockhost, $sock->sockport);
my @clone = qw(git clone -q -s --bare);
--
EW
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 2/2] daemon: warn on inheriting blocking listeners
2019-06-30 22:19 [PATCH 0/2] warn on inheriting blocking sockets Eric Wong
2019-06-30 22:19 ` [PATCH 1/2] tests: common tcp_server and unix_server helpers Eric Wong
@ 2019-06-30 22:19 ` Eric Wong
1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2019-06-30 22:19 UTC (permalink / raw)
To: meta
For users relying on socket activation via service manager (e.g.
systemd) and running multiple service instances (@1, @2),
we need to ensure configuration of the socket is NonBlocking.
Otherwise, service managers such as systemd may clear the
O_NONBLOCK flag for a small window where accept/accept4
blocks:
public-inbox-nntpd@1 |systemd |public-inbox-nntpd@2
--------------------------+----------------+--------------------
F_SETFL,O_NONBLOCK|O_RDWR | | (not running, yet)
|F_SETFL, O_RDWR |
|fork+exec @2... |
accept(...) # blocks! | |(started by systemd)
| |F_SETFL,O_NONBLOCK|O_RDWR
| |accept(...) non-blocking
It's a very small window where O_NONBLOCK can be cleared,
but it exists, and I finally hit it after many years.
---
lib/PublicInbox/Daemon.pm | 10 +++++++++-
lib/PublicInbox/Listener.pm | 1 -
t/common.perl | 7 +++++--
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 2b7ac266..2046a7f5 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -155,9 +155,9 @@ sub daemon_prepare ($) {
my $s = eval { $sock_pkg->new(%o) };
warn "error binding $l: $! ($@)\n" unless $s;
umask $prev;
-
if ($s) {
$listener_names{sockname($s)} = $s;
+ $s->blocking(0);
push @listeners, $s;
}
}
@@ -363,6 +363,14 @@ sub inherit () {
foreach my $fd (3..$end) {
my $s = IO::Handle->new_from_fd($fd, 'r');
if (my $k = sockname($s)) {
+ if ($s->blocking) {
+ $s->blocking(0);
+ warn <<"";
+Inherited socket (fd=$fd) is blocking, making it non-blocking.
+Set 'NonBlocking = true' in the systemd.service unit to avoid stalled
+processes when multiple service instances start.
+
+ }
$listener_names{$k} = $s;
push @rv, $s;
} else {
diff --git a/lib/PublicInbox/Listener.pm b/lib/PublicInbox/Listener.pm
index 594dabb8..60428934 100644
--- a/lib/PublicInbox/Listener.pm
+++ b/lib/PublicInbox/Listener.pm
@@ -16,7 +16,6 @@ sub new ($$$) {
setsockopt($s, SOL_SOCKET, SO_KEEPALIVE, 1);
setsockopt($s, IPPROTO_TCP, TCP_NODELAY, 1); # ignore errors on non-TCP
listen($s, 1024);
- IO::Handle::blocking($s, 0);
my $self = fields::new($class);
$self->SUPER::new($s, EPOLLIN|EPOLLET|EPOLLEXCLUSIVE);
$self->{post_accept} = $cb;
diff --git a/t/common.perl b/t/common.perl
index 3f05b68a..91d65c5f 100644
--- a/t/common.perl
+++ b/t/common.perl
@@ -24,15 +24,18 @@ sub tcp_server () {
Proto => 'tcp',
Type => Socket::SOCK_STREAM(),
Listen => 1024,
+ Blocking => 0,
)
}
sub unix_server ($) {
- IO::Socket::UNIX->new(
+ my $s = IO::Socket::UNIX->new(
Listen => 1024,
Type => Socket::SOCK_STREAM(),
Local => $_[0],
- )
+ );
+ $s->blocking(0);
+ $s;
}
sub spawn_listener {
--
EW
^ permalink raw reply related [flat|nested] 3+ messages in thread