user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 2/2] daemon: warn on inheriting blocking listeners
Date: Sun, 30 Jun 2019 22:19:39 +0000	[thread overview]
Message-ID: <20190630221939.9885-3-e@80x24.org> (raw)
In-Reply-To: <20190630221939.9885-1-e@80x24.org>

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


      parent reply	other threads:[~2019-06-30 22:19 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://public-inbox.org/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190630221939.9885-3-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).