From: Eric Wong Date: Tue, 20 Aug 2024 10:35:19 +0000 (+0000) Subject: sigfd: call normal Perl %SIG handlers X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cd85a76f31b8d93ff686c1f89e476daf269f1a90;p=thirdparty%2Fpublic-inbox.git sigfd: call normal Perl %SIG handlers 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 makes it easier to allow existing synchronous code (e.g. NetReader using Mail::IMAPClient or Net::NNTP) to add explicit points where pending signals can be checked. Additionally, it allows the `DEFAULT' (SIG_DFL) signal handler to fire when there's no Perl subroutine to register. Finally, this also allows us to rely on the OS + Perl itself to dispatch signal handlers on kevent-based systems (and avoid redundant dispatch due to our (previous) Linux-centric API). It makes Linux signalfd the only system where we'd need to dispatch %SIG callbacks ourselves. --- diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm index f807c6269..bb45ec99e 100644 --- a/lib/PublicInbox/DS.pm +++ b/lib/PublicInbox/DS.pm @@ -248,7 +248,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): $!"; } @@ -259,6 +259,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 for how to exit the loop. sub event_loop (;$$) { @@ -266,17 +273,15 @@ sub event_loop (;$$) { $Poller //= _InitPoller(); require PublicInbox::Sigfd if $sig; my $sigfd = $sig ? PublicInbox::Sigfd->new($sig) : undef; - if ($sigfd && $sigfd->{is_kq}) { - my $tmp = allowset($sig); - local @SIG{keys %$sig} = values(%$sig); - sig_setmask($tmp, my $old = POSIX::SigSet->new); + local $SIG{PIPE} = 'IGNORE'; + local @SIG{keys %$sig} = values(%$sig) if $sig; + if ($sigfd && $sigfd->{kq_sigs}) { # Unlike Linux signalfd, EVFILT_SIGNAL can't handle # signals received before the filter is created, # so we peek at signals here. - sig_setmask($old); + my $restore = allow_sigs keys %$sig; + select undef, undef, undef, 0; # check sigs } - 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: diff --git a/lib/PublicInbox/Sigfd.pm b/lib/PublicInbox/Sigfd.pm index b8a1ddfb1..128d933e2 100644 --- a/lib/PublicInbox/Sigfd.pm +++ b/lib/PublicInbox/Sigfd.pm @@ -8,27 +8,31 @@ 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); + $self->{kq_sigs} = [ keys %$sig ]; } else { return; # wake up every second to check for signals } $self->SUPER::new($io, EPOLLIN | EPOLLET); - $self->{is_kq} = 1 if tied(*$io); $self; } @@ -37,13 +41,26 @@ sub wait_once ($) { my ($self) = @_; # 128 == sizeof(struct signalfd_siginfo) my $r = sysread($self->{sock}, my $buf, 128 * 64); - if (defined($r)) { + if ($self->{kq_sigs}) { + # kqueue doesn't consume signals the same way signalfd does, + # so the OS + Perl can make calls for us: + my $restore = PublicInbox::DS::allow_sigs @{$self->{kq_sigs}}; + select undef, undef, undef, 0; # checks signals + } elsif (defined($r)) { # Linux signalfd 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); + } # undef } } $r; diff --git a/t/sigfd.t b/t/sigfd.t index 9a7b947db..eab85ed74 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 }) { @@ -23,7 +24,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 +45,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 +62,39 @@ 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'); + + 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($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); } - 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;