* [PATCH 2/2] daemon: warn on inheriting blocking listeners
2019-06-30 22:19 6% [PATCH 0/2] warn on inheriting blocking sockets Eric Wong
@ 2019-06-30 22:19 7% ` Eric Wong
0 siblings, 0 replies; 2+ results
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 [relevance 7%]
* [PATCH 0/2] warn on inheriting blocking sockets
@ 2019-06-30 22:19 6% Eric Wong
2019-06-30 22:19 7% ` [PATCH 2/2] daemon: warn on inheriting blocking listeners Eric Wong
0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2019-06-30 22:19 UTC (permalink / raw)
To: meta
A followup to https://public-inbox.org/meta/20190630074146.GA16199@dcvr/
("examples/*@.service: sockets MUST be NonBlocking")
1/2 is a long-needed cleanup patch, 2/2 is what matters to users.
Yeah, I'm shocked it's taken as long as it has to notice this
oversight, I completely missed that systemd would clear O_NONBLOCK
before spawning new instances.
I kinda wish accept4 would take SOCK_DONTWAIT (and maybe
SOCK_MUSTWAIT), but I also don't want to bloat the kernel
even more.
Eric Wong (2):
tests: common tcp_server and unix_server helpers
daemon: warn on inheriting blocking listeners
lib/PublicInbox/Daemon.pm | 10 +++++++++-
lib/PublicInbox/Listener.pm | 1 -
t/common.perl | 21 +++++++++++++++++++++
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 +--------
13 files changed, 43 insertions(+), 88 deletions(-)
--
EW
^ permalink raw reply [relevance 6%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2019-06-30 22:19 6% [PATCH 0/2] warn on inheriting blocking sockets Eric Wong
2019-06-30 22:19 7% ` [PATCH 2/2] daemon: warn on inheriting blocking listeners Eric Wong
Code repositories for project(s) associated with this public inbox
https://80x24.org/public-inbox.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).