[PATCH 1/4] sigfd: call normally registered %SIG handlers

[PATCH 1/4] sigfd: call normally registered %SIG handlers

[Eric Wong]

Instead of storing our own mapping of signal handler callbacks,
rely on the standard %SIG hash table which can be arbitrarily
updated from anywhere.

This is a stopgap to allow existing synchronous code (e.g.
NetReader using Mail::IMAPClient or Net::NNTP) to add explicit
calls to check for pending signals.
---
 lib/PublicInbox/DS.pm    | 11 +++++++++--
 lib/PublicInbox/Sigfd.pm | 34 +++++++++++++++++++++++-----------
 t/sigfd.t                | 24 +++++++-----------------
 3 files changed, 39 insertions(+), 30 deletions(-)

diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index a6fec954..3af01af5 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -247,7 +247,7 @@ sub sigset_prep ($$$) {
 	my ($sig, $init, $each) = @_; # $sig: { signame => whatever }
 	my $ret = POSIX::SigSet->new;
 	$ret->$init or die "$init: $!";
-	for my $s (keys %$sig) {
+	for my $s (ref($sig) eq 'HASH' ? keys(%$sig) : @$sig) {
 		my $num = $SIGNUM{$s} // POSIX->can("SIG$s")->();
 		$ret->$each($num) or die "$each ($s => $num): $!";
 	}
@@ -258,6 +258,13 @@ sub sigset_prep ($$$) {
 sub allowset ($) { sigset_prep $_[0], 'fillset', 'delset' }
 sub unblockset ($) { sigset_prep $_[0], 'emptyset', 'addset' }
 
+sub allow_sigs (@) {
+	my @signames = @_;
+	my $tmp = allowset(\@signames);
+	sig_setmask($tmp, my $old = POSIX::SigSet->new);
+	on_destroy \&sig_setmask, $old;
+}
+
 # Start processing IO events. In most daemon programs this never exits. See
 # C<post_loop_do> for how to exit the loop.
 sub event_loop (;$$) {
@@ -274,7 +281,7 @@ sub event_loop (;$$) {
 		# so we peek at signals here.
 		sig_setmask($old);
 	}
-	local @SIG{keys %$sig} = values(%$sig) if $sig && !$sigfd;
+	local @SIG{keys %$sig} = values(%$sig) if $sig;
 	local $SIG{PIPE} = 'IGNORE';
 	if (!$sigfd && $sig) {
 		# wake up every second to accept signals if we don't
diff --git a/lib/PublicInbox/Sigfd.pm b/lib/PublicInbox/Sigfd.pm
index b8a1ddfb..ef88012e 100644
--- a/lib/PublicInbox/Sigfd.pm
+++ b/lib/PublicInbox/Sigfd.pm
@@ -8,22 +8,26 @@ use v5.12;
 use parent qw(PublicInbox::DS);
 use PublicInbox::Syscall qw(signalfd EPOLLIN EPOLLET %SIGNUM);
 use POSIX ();
+use autodie qw(kill open);
+my @num2name;
 
 # returns a coderef to unblock signals if neither signalfd or kqueue
 # are available.
 sub new {
 	my ($class, $sig) = @_;
-	my %signo = map {;
-		# $num => [ $cb, $signame ];
-		($SIGNUM{$_} // POSIX->can("SIG$_")->()) => [ $sig->{$_}, $_ ]
-	} keys %$sig;
-	my $self = bless { sig => \%signo }, $class;
+	my @signo;
+	for my $name (keys %$sig) {
+		my $num = $SIGNUM{$name} // POSIX->can("SIG$name")->();
+		push @signo, $num;
+		$num2name[$num] //= $name;
+	}
+	my $self = bless {}, $class;
 	my $io;
-	my $fd = signalfd([keys %signo]);
+	my $fd = signalfd(\@signo);
 	if (defined $fd && $fd >= 0) {
-		open($io, '+<&=', $fd) or die "open: $!";
+		open $io, '+<&=', $fd;
 	} elsif (eval { require PublicInbox::DSKQXS }) {
-		$io = PublicInbox::DSKQXS->signalfd([keys %signo]);
+		$io = PublicInbox::DSKQXS->signalfd(\@signo);
 	} else {
 		return; # wake up every second to check for signals
 	}
@@ -41,9 +45,17 @@ sub wait_once ($) {
 		my $nr = $r / 128 - 1; # $nr may be -1
 		for my $off (0..$nr) {
 			# the first uint32_t of signalfd_siginfo: ssi_signo
-			my $signo = unpack('L', substr($buf, 128 * $off, 4));
-			my ($cb, $signame) = @{$self->{sig}->{$signo}};
-			$cb->($signame) if $cb ne 'IGNORE';
+			my $num = unpack('L', substr($buf, 128 * $off, 4));
+			my $name = $num2name[$num];
+			my $cb = $SIG{$name} || 'IGNORE';
+			if ($cb eq 'DEFAULT') {
+				my $restore = PublicInbox::DS::allow_sigs $name;
+				kill $name, $$;
+				select undef, undef, undef, 0; # checks signals
+				# $restore fires
+			} elsif (ref $cb) {
+				$cb->($name);
+			}
 		}
 	}
 	$r;
diff --git a/t/sigfd.t b/t/sigfd.t
index 9a7b947d..aada275d 100644
--- a/t/sigfd.t
+++ b/t/sigfd.t
@@ -23,7 +23,7 @@ SKIP: {
 	local $SIG{INT} = sub { $hit->{INT}->{normal}++ };
 	local $SIG{WINCH} = sub { $hit->{WINCH}->{normal}++ };
 	for my $s (qw(USR2 HUP TERM INT WINCH)) {
-		$sig->{$s} = sub { $hit->{$s}->{sigfd}++ };
+		$sig->{$s} = sub { die "SHOULD NOT BE CALLED ($s)" }
 	}
 	kill 'USR2', $$ or die "kill $!";
 	ok(!defined($hit->{USR2}), 'no USR2 yet') or diag explain($hit);
@@ -44,16 +44,13 @@ SKIP: {
 		is(select($rvec, undef, undef, undef), 1, 'select() works');
 		ok($sigfd->wait_once, 'wait_once reported success');
 		for my $s (qw(HUP INT)) {
-			is($hit->{$s}->{sigfd}, 1, "sigfd fired $s");
-			is($hit->{$s}->{normal}, undef,
-				"normal \$SIG{$s} not fired");
+			is($hit->{$s}->{normal}, 1, "sigfd fired $s");
 		}
 		SKIP: {
 			skip 'Linux sigfd-only behavior', 1 if !$linux_sigfd;
-			is($hit->{USR2}->{sigfd}, 1,
+			is($hit->{USR2}->{normal}, 1,
 				'USR2 sent before signalfd created received');
 		}
-		ok(!$hit->{USR2}->{normal}, 'USR2 not fired normally');
 		PublicInbox::DS->Reset;
 		$sigfd = undef;
 
@@ -64,26 +61,19 @@ SKIP: {
 		kill('HUP', $$) or die "kill $!";
 		local @PublicInbox::DS::post_loop_do = (sub {}); # loop once
 		PublicInbox::DS::event_loop();
-		is($hit->{HUP}->{sigfd}, 2, 'HUP sigfd fired in event loop') or
+		is($hit->{HUP}->{normal}, 2, 'HUP sigfd fired in event loop') or
 			diag explain($hit); # sometimes fails on FreeBSD 11.x
 		kill('TERM', $$) or die "kill $!";
 		kill('HUP', $$) or die "kill $!";
 		PublicInbox::DS::event_loop();
 		PublicInbox::DS->Reset;
-		is($hit->{TERM}->{sigfd}, 1, 'TERM sigfd fired in event loop');
-		is($hit->{HUP}->{sigfd}, 3, 'HUP sigfd fired in event loop');
-		ok($hit->{WINCH}->{sigfd}, 'WINCH sigfd fired in event loop');
+		is($hit->{TERM}->{normal}, 1, 'TERM sigfd fired in event loop');
+		is($hit->{HUP}->{normal}, 3, 'HUP sigfd fired in event loop');
+		ok($hit->{WINCH}->{normal}, 'WINCH sigfd fired in event loop');
 	} else {
 		skip('signalfd disabled?', 10);
 	}
-	ok(!$hit->{USR2}->{normal}, 'USR2 still not fired normally');
 	PublicInbox::DS::sig_setmask($old);
-	SKIP: {
-		($has_sigfd && !$linux_sigfd) or
-			skip 'EVFILT_SIGNAL-only behavior check', 1;
-		is($hit->{USR2}->{normal}, 1,
-			"USR2 fired normally after unblocking on $^O");
-	}
 }
 
 done_testing;

[PATCH 2/4] lei: simplify forced signal check

[Eric Wong]

There's no need to loop since a select call is enough to force
signal handlers to be fired.
---
 script/lei | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/script/lei b/script/lei
index 087afc33..6e866b22 100755
--- a/script/lei
+++ b/script/lei
@@ -139,6 +139,6 @@ while (1) {
 $sigchld->();
 if (my $sig = ($x_it_code & 127)) {
 	kill $sig, $$;
-	sleep(1) while 1; # no self-pipe/signalfd, here, so we loop
+	select undef, undef, undef, 0; # for kernel to act on signal
 }
 exit($x_it_code >> 8);

[PATCH 3/4] lei: convert allow Ctrl-C to interrupt IMAP+NNTP reads

[Eric Wong]

---
 lib/PublicInbox/NetReader.pm | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index ec18818b..41d4b190 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -7,6 +7,7 @@ use v5.12;
 use parent qw(Exporter PublicInbox::IPC);
 use PublicInbox::Eml;
 use PublicInbox::Config;
+use PublicInbox::DS;
 our %IMAPflags2kw = map {; "\\\u$_" => $_ } qw(seen answered flagged draft);
 $IMAPflags2kw{'$Forwarded'} = 'forwarded';  # RFC 5550
 
@@ -61,6 +62,7 @@ sub mic_new ($$$$) {
 	my %mic_arg = (%$mic_arg, Keepalive => 1);
 	my $sa = $self->{cfg_opt}->{$sec}->{-proxy_cfg} || $self->{-proxy_cli};
 	my ($mic, $s, $t);
+	my $restore = PublicInbox::DS::allow_sigs qw(INT QUIT TERM);
 	if ($sa) {
 		# this `require' needed for worker[1..Inf], since socks_args
 		# only got called in worker[0]
@@ -226,6 +228,7 @@ sub nn_new ($$$$) {
 		($Net_NNTP, $new) = qw(PublicInbox::NetNNTPSocks new_socks);
 		$nn_arg->{SocksDebug} = 1 if $nn_arg->{Debug};
 	}
+	my $restore = PublicInbox::DS::allow_sigs qw(INT QUIT TERM);
 	do {
 		$! = 0;
 		$nn = $Net_NNTP->$new(%$nn_arg);
@@ -567,11 +570,13 @@ sub each_old_flags ($$$$) {
 	for (my $n = 1; $n <= $l_uid; $n += $bs) {
 		my $end = $n + $bs;
 		$end = $l_uid if $end > $l_uid;
+		my $restore = PublicInbox::DS::allow_sigs qw(INT QUIT TERM);
 		my $r = $mic->fetch_hash("$n:$end", 'FLAGS');
 		if (!$r) {
 			return if $!{EINTR} && $self->{quit};
 			return "E: $uri UID FETCH $n:$end error: $!";
 		}
+		undef $restore;
 		while (my ($uid, $per_uid) = each %$r) {
 			my $kw = flags2kw($self, $uri, $uid, $per_uid->{FLAGS})
 				// next;
@@ -610,12 +615,14 @@ sub _imap_fetch_bodies ($$$$) {
 		my @batch = splice(@$uids, 0, $bs);
 		my $batch = join(',', @batch);
 		local $0 = "UID:$batch $mbx $sec";
+		my $restore = PublicInbox::DS::allow_sigs qw(INT QUIT TERM);
 		my $r = $mic->fetch_hash($batch, $req, 'FLAGS');
 		unless ($r) { # network error?
 			last if $!{EINTR} && $self->{quit};
 			$err = "E: $uri UID FETCH $batch error: $!";
 			last;
 		}
+		undef $restore;
 		for my $uid (@batch) {
 			# messages get deleted, so holes appear
 			my $per_uid = delete $r->{$uid} // next;
@@ -639,6 +646,7 @@ sub _imap_fetch_all ($$$) {
 	# we need to check for mailbox writability to see if we care about
 	# FLAGS from already-imported messages.
 	my $cmd = $self->folder_select;
+	my $restore = PublicInbox::DS::allow_sigs qw(INT QUIT TERM);
 	$mic->$cmd($mbx) or return "E: \U$cmd\E $mbx ($sec) failed: $!";
 
 	my ($r_uidval, $r_uidnext, $perm_fl);
@@ -656,6 +664,7 @@ sub _imap_fetch_all ($$$) {
 E: $orig_uri UIDVALIDITY mismatch (got $r_uidval)
 EOF
 
+	undef $restore;
 	my $uri = $orig_uri->clone;
 	my $single_uid = $uri->uid;
 	my ($itrk, $l_uid, $l_uidval) = itrk_last($self, $uri, $r_uidval, $mic);
@@ -780,6 +789,7 @@ sub nn_get {
 	my $nntp_cfg = $self->{cfg_opt}->{$sec};
 	$nn = nn_new($self, $nn_arg, $nntp_cfg, $uri) or return;
 	if (my $postconn = $nntp_cfg->{-postconn}) {
+		my $restore = PublicInbox::DS::allow_sigs qw(INT QUIT TERM);
 		for my $m_arg (@$postconn) {
 			my ($method, @args) = @$m_arg;
 			$nn->$method(@args) and next;
@@ -794,11 +804,13 @@ sub _nntp_fetch_all ($$$) {
 	my ($self, $nn, $uri) = @_;
 	my ($group, $num_a, $num_b) = $uri->group;
 	my $sec = uri_section($uri);
+	my $restore = PublicInbox::DS::allow_sigs qw(INT QUIT TERM);
 	my ($nr, $beg, $end) = $nn->group($group);
 	unless (defined($nr)) {
 		my $msg = ndump($nn->message);
 		return "E: GROUP $group <$sec> $msg";
 	}
+	undef $restore;
 	(defined($num_a) && defined($num_b) && $num_a > $num_b) and
 		return "E: $uri: backwards range: $num_a > $num_b";
 	if (defined($num_a)) { # no article numbers in mail_sync.sqlite3
@@ -831,6 +843,7 @@ sub _nntp_fetch_all ($$$) {
 			$itrk->update_last(0, $last_art) if $itrk;
 			$n = $self->{max_batch};
 		}
+		$restore = PublicInbox::DS::allow_sigs qw(INT QUIT TERM);
 		my $raw = $nn->article($art);
 		unless (defined($raw)) {
 			my $msg = ndump($nn->message);
@@ -842,6 +855,7 @@ sub _nntp_fetch_all ($$$) {
 				next;
 			}
 		}
+		undef $restore;
 		$raw = join('', @$raw);
 		$raw =~ s/\r\n/\n/sg;
 		my ($eml_cb, @args) = @{$self->{eml_each}};

[PATCH 4/4] t/sigfd: more tests

[Eric Wong]

---
 t/sigfd.t | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/t/sigfd.t b/t/sigfd.t
index aada275d..f1443dd2 100644
--- a/t/sigfd.t
+++ b/t/sigfd.t
@@ -8,6 +8,7 @@ use Errno qw(ENOSYS);
 require_ok 'PublicInbox::Sigfd';
 use PublicInbox::DS;
 my ($linux_sigfd, $has_sigfd);
+use autodie qw(kill);
 
 SKIP: {
 	if ($^O ne 'linux' && !eval { require IO::KQueue }) {
@@ -70,6 +71,26 @@ SKIP: {
 		is($hit->{TERM}->{normal}, 1, 'TERM sigfd fired in event loop');
 		is($hit->{HUP}->{normal}, 3, 'HUP sigfd fired in event loop');
 		ok($hit->{WINCH}->{normal}, 'WINCH sigfd fired in event loop');
+
+		my $restore = PublicInbox::DS::allow_sigs 'HUP';
+		kill 'HUP', $$;
+		select undef, undef, undef, 0;
+		is $hit->{HUP}->{normal}, 4, 'HUP sigfd fired after allow_sigs';
+
+		undef $restore;
+		kill 'HUP', $$;
+		vec(my $rvec = '', fileno($nbsig->{sock}), 1) = 1;
+		ok select($rvec, undef, undef, 1),
+			'select reports sigfd readiness';
+		is $hit->{HUP}->{normal}, 4, 'HUP not fired when sigs blocked';
+		$nbsig->event_step;
+		is $hit->{HUP}->{normal}, 5, 'HUP fires only on ->event_step';
+
+		kill 'HUP', $$;
+		is $hit->{HUP}->{normal}, 5, 'HUP not fired, yet';
+		$restore = PublicInbox::DS::allow_sigs 'HUP';
+		select(undef, undef, undef, 0);
+		is $hit->{HUP}->{normal}, 6, 'HUP fires from allow_sigs';
 	} else {
 		skip('signalfd disabled?', 10);
 	}