From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, T_SCC_BODY_TEXT_LINE shortcircuit=no autolearn=ham autolearn_force=no version=3.4.6 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 0A2061F518 for ; Tue, 20 Aug 2024 10:35:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1724150123; bh=vg/nSFxsKv+hjVlf465YekOwdCXPPEbAFyjqOZkzQv0=; h=From:To:Subject:Date:In-Reply-To:References:From; b=UKMtnIJ1QFRg8FKisxYfHWHwj/jvOa0rm6SRiDodDnxeLLxYO1N4nUwgTSYkCum73 FWktPoaNr6GtWkCwzJYDYW+03v5vFrX6ImLh92D6uHRlzFPZozEC7lN8KV+DNPes+X Lc2x+cKhEkWk9Yq+eHjrErP6iqOiG5E8rvQLgsKk= From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 3/5] sigfd: call normal Perl %SIG handlers Date: Tue, 20 Aug 2024 10:35:19 +0000 Message-ID: <20240820103522.3548609-4-e@80x24.org> In-Reply-To: <20240820103522.3548609-1-e@80x24.org> References: <20240820103522.3548609-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: 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. --- lib/PublicInbox/DS.pm | 21 ++++++++++++------- lib/PublicInbox/Sigfd.pm | 43 ++++++++++++++++++++++++++------------ t/sigfd.t | 45 +++++++++++++++++++++++++--------------- 3 files changed, 71 insertions(+), 38 deletions(-) diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm index f807c626..bb45ec99 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 b8a1ddfb..128d933e 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 9a7b947d..eab85ed7 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;