From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 54D6A1F4B7 for ; Sun, 30 Jun 2019 22:19:40 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 2/2] daemon: warn on inheriting blocking listeners Date: Sun, 30 Jun 2019 22:19:39 +0000 Message-Id: <20190630221939.9885-3-e@80x24.org> In-Reply-To: <20190630221939.9885-1-e@80x24.org> References: <20190630221939.9885-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: 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