ds: simplify signalfd use

Since signalfd is often combined with our event loop, give it a
convenient API and reduce the code duplication required to use it.

EventLoop is replaced with ::event_loop to allow consistent
parameter passing and avoid needlessly passing the package name
on stack.

We also avoid exporting SFD_NONBLOCK since it's the only flag we
support.  There's no sense in having the memory overhead of a
constant function when it's in cold code.

13 files changed, 65 insertions, 90 deletions

diff --git a/lib/PublicInbox/ConfigIter.pm b/lib/PublicInbox/ConfigIter.pm
index 24cb09bf..14fcef83 100644
--- a/lib/PublicInbox/ConfigIter.pm
+++ b/lib/PublicInbox/ConfigIter.pm
@@ -1,7 +1,7 @@
 # Copyright (C) 2020-2021 all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
-# Intended for PublicInbox::DS->EventLoop in read-only daemons
+# Intended for PublicInbox::DS::event_loop in read-only daemons
 # to avoid each_inbox() monopolizing the event loop when hundreds/thousands
 # of inboxes are in play.
 package PublicInbox::ConfigIter;
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 37cd6087..ba6c74d0 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -155,13 +155,6 @@ sub _InitPoller
     }
 }
 
-=head2 C<< CLASS->EventLoop() >>
-
-Start processing IO events. In most daemon programs this never exits. See
-C<PostLoopCallback> below for how to exit the loop.
-
-=cut
-
 sub now () { clock_gettime(CLOCK_MONOTONIC) }
 
 sub next_tick () {
@@ -277,26 +270,41 @@ sub PostEventLoop () {
         $PostLoopCallback ? $PostLoopCallback->(\%DescriptorMap) : 1;
 }
 
-sub EventLoop {
-    $Epoll //= _InitPoller();
-    local $in_loop = 1;
-    my @events;
-    do {
-        my $timeout = RunTimers();
-
-        # get up to 1000 events
-        epoll_wait($Epoll, 1000, $timeout, \@events);
-        for my $fd (@events) {
-            # it's possible epoll_wait returned many events, including some at the end
-            # that ones in the front triggered unregister-interest actions.  if we
-            # can't find the %sock entry, it's because we're no longer interested
-            # in that event.
-
-            # guard stack-not-refcounted w/ Carp + @DB::args
-            my $obj = $DescriptorMap{$fd};
-            $obj->event_step;
-        }
-    } while (PostEventLoop());
+# Start processing IO events. In most daemon programs this never exits. See
+# C<PostLoopCallback> for how to exit the loop.
+sub event_loop (;$$) {
+        my ($sig, $oldset) = @_;
+        $Epoll //= _InitPoller();
+        require PublicInbox::Sigfd if $sig;
+        my $sigfd = PublicInbox::Sigfd->new($sig, 1) if $sig;
+        local @SIG{keys %$sig} = values(%$sig) if $sig && !$sigfd;
+        local $SIG{PIPE} = 'IGNORE';
+        if (!$sigfd && $sig) {
+                # wake up every second to accept signals if we don't
+                # have signalfd or IO::KQueue:
+                sig_setmask($oldset);
+                PublicInbox::DS->SetLoopTimeout(1000);
+        }
+        $_[0] = $sigfd = $sig = undef; # $_[0] == sig
+        local $in_loop = 1;
+        my @events;
+        do {
+                my $timeout = RunTimers();
+
+                # get up to 1000 events
+                epoll_wait($Epoll, 1000, $timeout, \@events);
+                for my $fd (@events) {
+                        # it's possible epoll_wait returned many events,
+                        # including some at the end that ones in the front
+                        # triggered unregister-interest actions.  if we can't
+                        # find the %sock entry, it's because we're no longer
+                        # interested in that event.
+
+                        # guard stack-not-refcounted w/ Carp + @DB::args
+                        my $obj = $DescriptorMap{$fd};
+                        $obj->event_step;
+                }
+        } while (PostEventLoop());
 }
 
 =head2 C<< CLASS->SetPostLoopCallback( CODEREF ) >>
@@ -326,7 +334,7 @@ sub SetPostLoopCallback {
 =head2 C<< CLASS->new( $socket ) >>
 
 Create a new PublicInbox::DS subclass object for the given I<socket> which will
-react to events on it during the C<EventLoop>.
+react to events on it during the C<event_loop>.
 
 This is normally (always?) called from your subclass via:
 
diff --git a/lib/PublicInbox/DSKQXS.pm b/lib/PublicInbox/DSKQXS.pm
index acc31d9b..eccfa56d 100644
--- a/lib/PublicInbox/DSKQXS.pm
+++ b/lib/PublicInbox/DSKQXS.pm
@@ -18,7 +18,7 @@ use Symbol qw(gensym);
 use IO::KQueue;
 use Errno qw(EAGAIN);
 use PublicInbox::Syscall qw(EPOLLONESHOT EPOLLIN EPOLLOUT EPOLLET
-        EPOLL_CTL_ADD EPOLL_CTL_MOD EPOLL_CTL_DEL SFD_NONBLOCK);
+        EPOLL_CTL_ADD EPOLL_CTL_MOD EPOLL_CTL_DEL);
 our @EXPORT_OK = qw(epoll_ctl epoll_wait);
 
 sub EV_DISPATCH () { 0x0080 }
@@ -48,16 +48,16 @@ sub new {
 # It's wasteful in that it uses another FD, but it simplifies
 # our epoll-oriented code.
 sub signalfd {
-        my ($class, $signo, $flags) = @_;
+        my ($class, $signo, $nonblock) = @_;
         my $sym = gensym;
-        tie *$sym, $class, $signo, $flags; # calls TIEHANDLE
+        tie *$sym, $class, $signo, $nonblock; # calls TIEHANDLE
         $sym
 }
 
 sub TIEHANDLE { # similar to signalfd()
-        my ($class, $signo, $flags) = @_;
+        my ($class, $signo, $nonblock) = @_;
         my $self = $class->new;
-        $self->{timeout} = ($flags & SFD_NONBLOCK) ? 0 : -1;
+        $self->{timeout} = $nonblock ? 0 : -1;
         my $kq = $self->{kq};
         $kq->EV_SET($_, EVFILT_SIGNAL, EV_ADD) for @$signo;
         $self;
diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 24dc7791..5be474fa 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -15,7 +15,6 @@ use Socket qw(IPPROTO_TCP SOL_SOCKET);
 STDOUT->autoflush(1);
 STDERR->autoflush(1);
 use PublicInbox::DS qw(now);
-use PublicInbox::Syscall qw(SFD_NONBLOCK);
 require PublicInbox::Listener;
 use PublicInbox::EOFpipe;
 use PublicInbox::Sigfd;
@@ -513,7 +512,7 @@ EOF
                 },
                 CHLD => \&reap_children,
         };
-        my $sigfd = PublicInbox::Sigfd->new($sig, 0);
+        my $sigfd = PublicInbox::Sigfd->new($sig);
         local @SIG{keys %$sig} = values(%$sig) unless $sigfd;
         PublicInbox::DS::sig_setmask($oldset) if !$sigfd;
         while (1) { # main loop
@@ -630,20 +629,11 @@ sub daemon_loop ($$$$) {
                 # this calls epoll_create:
                 PublicInbox::Listener->new($_, $tls_cb || $post_accept)
         } @listeners;
-        my $sigfd = PublicInbox::Sigfd->new($sig, SFD_NONBLOCK);
-        local @SIG{keys %$sig} = values(%$sig) unless $sigfd;
-        if (!$sigfd) {
-                # wake up every second to accept signals if we don't
-                # have signalfd or IO::KQueue:
-                PublicInbox::DS::sig_setmask($oldset);
-                PublicInbox::DS->SetLoopTimeout(1000);
-        }
-        PublicInbox::DS->EventLoop;
+        PublicInbox::DS::event_loop($sig, $oldset);
 }
 
 sub run ($$$;$) {
         my ($default, $refresh, $post_accept, $tlsd) = @_;
-        local $SIG{PIPE} = 'IGNORE';
         daemon_prepare($default);
         my $af_default = $default =~ /:8080\z/ ? 'httpready' : undef;
         my $for_destroy = daemonize();
diff --git a/lib/PublicInbox/ExtMsg.pm b/lib/PublicInbox/ExtMsg.pm
index c134de55..72cae005 100644
--- a/lib/PublicInbox/ExtMsg.pm
+++ b/lib/PublicInbox/ExtMsg.pm
@@ -150,7 +150,7 @@ sub ext_msg {
         };
 }
 
-# called via PublicInbox::DS->EventLoop
+# called via PublicInbox::DS::event_loop
 sub event_step {
         my ($ctx, $sync) = @_;
         # can't find a partial match in current inbox, try the others:
diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 6b29789a..c34225b2 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -1305,19 +1305,11 @@ sub eidx_watch { # public-inbox-extindex --watch main loop
         };
         my $quit = PublicInbox::SearchIdx::quit_cb($sync);
         $sig->{QUIT} = $sig->{INT} = $sig->{TERM} = $quit;
-        my $sigfd = PublicInbox::Sigfd->new($sig,
-                                        $PublicInbox::Syscall::SFD_NONBLOCK);
-        @SIG{keys %$sig} = values(%$sig) if !$sigfd;
         local $self->{-watch_sync} = $sync; # for ->on_inbox_unlock
-        if (!$sigfd) {
-                # wake up every second to accept signals if we don't
-                # have signalfd or IO::KQueue:
-                PublicInbox::DS::sig_setmask($oldset);
-                PublicInbox::DS->SetLoopTimeout(1000);
-        }
         PublicInbox::DS->SetPostLoopCallback(sub { !$sync->{quit} });
         $pr->("initial scan complete, entering event loop\n") if $pr;
-        PublicInbox::DS->EventLoop; # calls InboxIdle->event_step
+        # calls InboxIdle->event_step:
+        PublicInbox::DS::event_loop($sig, $oldset);
         done($self);
 }
 
diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm
index 397774f9..c5695db1 100644
--- a/lib/PublicInbox/Gcf2Client.pm
+++ b/lib/PublicInbox/Gcf2Client.pm
@@ -57,7 +57,7 @@ sub gcf2_async ($$$;$) {
 # ensure PublicInbox::Git::cat_async_step never calls cat_async_retry
 sub alternates_changed {}
 
-# DS->EventLoop will call this
+# DS::event_loop will call this
 sub event_step {
         my ($self) = @_;
         $self->flush_write;
@@ -74,7 +74,7 @@ sub event_step {
 
 sub DESTROY {
         my ($self) = @_;
-        delete $self->{sock}; # if outside EventLoop
+        delete $self->{sock}; # if outside event_loop
         PublicInbox::Git::DESTROY($self);
 }
 
diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index 205b5b92..6c189b64 100644
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -251,7 +251,7 @@ sub wq_worker_loop ($$) {
         my $wqw = PublicInbox::WQWorker->new($self, $self->{-wq_s2});
         PublicInbox::WQWorker->new($self, $bcast2) if $bcast2;
         PublicInbox::DS->SetPostLoopCallback(sub { $wqw->{sock} });
-        PublicInbox::DS->EventLoop;
+        PublicInbox::DS::event_loop();
         PublicInbox::DS->Reset;
 }
 
@@ -353,7 +353,6 @@ sub _wq_worker_start ($$$$) {
                 delete @$self{qw(-wq_s1 -wq_ppid)};
                 $self->{-wq_worker_nr} =
                                 keys %{delete($self->{-wq_workers}) // {}};
-                $SIG{$_} = 'IGNORE' for (qw(PIPE));
                 $SIG{$_} = 'DEFAULT' for (qw(TTOU TTIN TERM QUIT INT CHLD));
                 local $0 = $one ? $self->{-wq_ident} :
                         "$self->{-wq_ident} $self->{-wq_worker_nr}";
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index df0bfab6..fd592358 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -18,8 +18,7 @@ use POSIX qw(strftime);
 use IO::Handle ();
 use Fcntl qw(SEEK_SET);
 use PublicInbox::Config;
-use PublicInbox::Syscall qw(SFD_NONBLOCK EPOLLIN EPOLLET);
-use PublicInbox::Sigfd;
+use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
 use PublicInbox::DS qw(now dwaitpid);
 use PublicInbox::Spawn qw(spawn popen_rd);
 use PublicInbox::Lock;
@@ -1291,23 +1290,11 @@ sub lazy_start {
                 USR1 => \&noop,
                 USR2 => \&noop,
         };
-        my $sigfd = PublicInbox::Sigfd->new($sig, SFD_NONBLOCK);
-        local @SIG{keys %$sig} = values(%$sig) unless $sigfd;
-        undef $sig;
-        local $SIG{PIPE} = 'IGNORE';
         require PublicInbox::DirIdle;
         local $dir_idle = PublicInbox::DirIdle->new([$sock_dir], sub {
                 # just rely on wakeup to hit PostLoopCallback set below
                 dir_idle_handler($_[0]) if $_[0]->fullname ne $path;
         }, 1);
-        if ($sigfd) {
-                undef $sigfd; # unref, already in DS::DescriptorMap
-        } else {
-                # wake up every second to accept signals if we don't
-                # have signalfd or IO::KQueue:
-                PublicInbox::DS::sig_setmask($oldset);
-                PublicInbox::DS->SetLoopTimeout(1000);
-        }
         PublicInbox::DS->SetPostLoopCallback(sub {
                 my ($dmap, undef) = @_;
                 if (@st = defined($path) ? stat($path) : ()) {
@@ -1344,7 +1331,7 @@ sub lazy_start {
         open STDERR, '>&STDIN' or die "redirect stderr failed: $!";
         open STDOUT, '>&STDIN' or die "redirect stdout failed: $!";
         # $daemon pipe to `lei' closed, main loop begins:
-        eval { PublicInbox::DS->EventLoop };
+        eval { PublicInbox::DS::event_loop($sig, $oldset) };
         warn "event loop error: $@\n" if $@;
         # exit() may trigger waitpid via various DESTROY, ensure interruptible
         PublicInbox::DS::sig_setmask($oldset);
diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm
index 7e50a59a..b1285eda 100644
--- a/lib/PublicInbox/Qspawn.pm
+++ b/lib/PublicInbox/Qspawn.pm
@@ -12,7 +12,7 @@
 # operate in.  This can be useful to ensure smaller inboxes can
 # be cloned while cloning of large inboxes is maxed out.
 #
-# This does not depend on the PublicInbox::DS->EventLoop or any
+# This does not depend on the PublicInbox::DS::event_loop or any
 # other external scheduling mechanism, you just need to call
 # start() and finish() appropriately. However, public-inbox-httpd
 # (which uses PublicInbox::DS)  will be able to schedule this
diff --git a/lib/PublicInbox/Sigfd.pm b/lib/PublicInbox/Sigfd.pm
index d91ea0e7..81e5a1b1 100644
--- a/lib/PublicInbox/Sigfd.pm
+++ b/lib/PublicInbox/Sigfd.pm
@@ -6,13 +6,13 @@
 package PublicInbox::Sigfd;
 use strict;
 use parent qw(PublicInbox::DS);
-use PublicInbox::Syscall qw(signalfd EPOLLIN EPOLLET SFD_NONBLOCK);
+use PublicInbox::Syscall qw(signalfd EPOLLIN EPOLLET);
 use POSIX ();
 
 # returns a coderef to unblock signals if neither signalfd or kqueue
 # are available.
 sub new {
-        my ($class, $sig, $flags) = @_;
+        my ($class, $sig, $nonblock) = @_;
         my %signo = map {;
                 my $cb = $sig->{$_};
                 # SIGWINCH is 28 on FreeBSD, NetBSD, OpenBSD
@@ -24,15 +24,15 @@ sub new {
         } keys %$sig;
         my $self = bless { sig => \%signo }, $class;
         my $io;
-        my $fd = signalfd(-1, [keys %signo], $flags);
+        my $fd = signalfd([keys %signo], $nonblock);
         if (defined $fd && $fd >= 0) {
                 open($io, '+<&=', $fd) or die "open: $!";
         } elsif (eval { require PublicInbox::DSKQXS }) {
-                $io = PublicInbox::DSKQXS->signalfd([keys %signo], $flags);
+                $io = PublicInbox::DSKQXS->signalfd([keys %signo], $nonblock);
         } else {
                 return; # wake up every second to check for signals
         }
-        if ($flags & SFD_NONBLOCK) { # it can go into the event loop
+        if ($nonblock) { # it can go into the event loop
                 $self->SUPER::new($io, EPOLLIN | EPOLLET);
         } else { # master main loop
                 $self->{sock} = $io;
diff --git a/lib/PublicInbox/Syscall.pm b/lib/PublicInbox/Syscall.pm
index a8a6f42a..7ab42911 100644
--- a/lib/PublicInbox/Syscall.pm
+++ b/lib/PublicInbox/Syscall.pm
@@ -22,7 +22,7 @@ our @EXPORT_OK = qw(epoll_ctl epoll_create epoll_wait
                   EPOLLIN EPOLLOUT EPOLLET
                   EPOLL_CTL_ADD EPOLL_CTL_DEL EPOLL_CTL_MOD
                   EPOLLONESHOT EPOLLEXCLUSIVE
-                  signalfd SFD_NONBLOCK);
+                  signalfd);
 our %EXPORT_TAGS = (epoll => [qw(epoll_ctl epoll_create epoll_wait
                              EPOLLIN EPOLLOUT
                              EPOLL_CTL_ADD EPOLL_CTL_DEL EPOLL_CTL_MOD
@@ -67,7 +67,6 @@ our (
      );
 
 my $SFD_CLOEXEC = 02000000; # Perl does not expose O_CLOEXEC
-sub SFD_NONBLOCK () { O_NONBLOCK }
 our $no_deprecated = 0;
 
 if ($^O eq "linux") {
@@ -266,14 +265,15 @@ sub epoll_wait_mod8 {
         }
 }
 
-sub signalfd ($$$) {
-        my ($fd, $signos, $flags) = @_;
+sub signalfd ($$) {
+        my ($signos, $nonblock) = @_;
         if ($SYS_signalfd4) {
                 my $set = POSIX::SigSet->new(@$signos);
-                syscall($SYS_signalfd4, $fd, "$$set",
+                syscall($SYS_signalfd4, -1, "$$set",
                         # $Config{sig_count} is NSIG, so this is NSIG/8:
                         int($Config{sig_count}/8),
-                        $flags|$SFD_CLOEXEC);
+                        # SFD_NONBLOCK == O_NONBLOCK for every architecture
+                        ($nonblock ? O_NONBLOCK : 0) |$SFD_CLOEXEC);
         } else {
                 $! = ENOSYS;
                 undef;
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index 0523ad03..c6bebce3 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -12,7 +12,6 @@ use PublicInbox::MdirReader;
 use PublicInbox::NetReader;
 use PublicInbox::Filter::Base qw(REJECT);
 use PublicInbox::Spamcheck;
-use PublicInbox::Sigfd;
 use PublicInbox::DS qw(now add_timer);
 use PublicInbox::MID qw(mids);
 use PublicInbox::ContentHash qw(content_hash);
@@ -570,7 +569,7 @@ sub watch { # main entry point
         }
         watch_fs_init($self) if $self->{mdre};
         PublicInbox::DS->SetPostLoopCallback(sub { !$self->quit_done });
-        PublicInbox::DS->EventLoop; # calls ->event_step
+        PublicInbox::DS::event_loop($sig, $oldset); # calls ->event_step
         _done_for_now($self);
 }