* [PATCH 00/10] signal-handling and *BSD fixes
@ 2023-09-04 10:35 Eric Wong
2023-09-04 10:35 ` [PATCH 01/10] ds: don't block important signals we don't use Eric Wong
` (9 more replies)
0 siblings, 10 replies; 11+ messages in thread
From: Eric Wong @ 2023-09-04 10:35 UTC (permalink / raw)
To: meta
kevent EVFILT_SIGNAL behaves differently than Linux signalfd
in that we can't rely on it to handle signals that were sent
before the existence of the kevent filter. Thus, [5/10] opens
a window before entering the event.
This difference between OSes was noticed due to lei tests
quickly starting read-only daemons and terminating them before
signal handlers were setup properly from the lack of
SCM_RIGHTS support on *BSDs lacking Inline::C.
Nevertheless [9/10] improves test dependency management to
ensure read-only daemons don't start all when SCM_RIGHTS
support is unavailable.
Patch [10/10] was originally far more aggressive in that it
kept all signals blocked across execve, but that's probably
unrealistic for real-world scenarios
7/10 to add TTOU/TTIN support to xap_helper was actually
the first developed in this series.
Lots more brewing in the portability department...
Eric Wong (10):
ds: don't block important signals we don't use
t/sigfd: test EVFILT_SIGNAL vs signalfd differences
t/sigfd: better checks related to SIGWINCH
update devel/syscall-list to devel/sysdefs-list
daemon: workaround pre-EVFILT_SIGNAL signals
watch: ensure children can use signal handlers
xap_helper: support SIGTTIN+SIGTTOU worker adjustments
xap_helper.h: include signal.h for sig* functions
tests: add `+SCM_RIGHTS' as a require_mods target
test_common: start_script: set default signals
MANIFEST | 2 +-
devel/{syscall-list => sysdefs-list} | 47 ++---
lib/PublicInbox/DS.pm | 38 +++-
lib/PublicInbox/IPC.pm | 2 +-
lib/PublicInbox/Sigfd.pm | 3 +-
lib/PublicInbox/Syscall.pm | 7 +-
lib/PublicInbox/TestCommon.pm | 26 ++-
lib/PublicInbox/Watch.pm | 7 +-
lib/PublicInbox/XapHelper.pm | 103 +++++++---
lib/PublicInbox/xap_helper.h | 283 +++++++++++++++++++++++----
script/public-inbox-watch | 4 +-
t/lei-import-nntp.t | 4 +-
t/lei.t | 3 +-
t/sigfd.t | 28 ++-
t/xap_helper.t | 59 ++++--
15 files changed, 489 insertions(+), 127 deletions(-)
rename devel/{syscall-list => sysdefs-list} (60%)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 01/10] ds: don't block important signals we don't use
2023-09-04 10:35 [PATCH 00/10] signal-handling and *BSD fixes Eric Wong
@ 2023-09-04 10:35 ` Eric Wong
2023-09-04 10:35 ` [PATCH 02/10] t/sigfd: test EVFILT_SIGNAL vs signalfd differences Eric Wong
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2023-09-04 10:35 UTC (permalink / raw)
To: meta
Don't block SIGABRT, SIGBUS, SIGFPE, SIGILL nor SIGSEGV since
blocking them can hide real bugs in our code or 3rd-party
libraries and executables.
We'll also leave SIGXCPU and SIGXFSZ unblocked since users
may've setup RLIMIT_CPU and RLIMIT_FSIZE, respectively.
---
lib/PublicInbox/DS.pm | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index e89dc430..97546016 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -193,10 +193,16 @@ sub RunTimers {
sub sig_setmask { sigprocmask(SIG_SETMASK, @_) or die "sigprocmask: $!" }
+our @UNBLOCKABLE = map { # ensure we detect bugs, HW problems and user rlimits
+ my $cb = POSIX->can("SIG$_");
+ my $num = $cb ? $cb->() : undef;
+ $num ? ($num) : ();
+} qw(ABRT BUS FPE ILL SEGV XCPU XFSZ);
+
sub block_signals { # anything in @_ stays unblocked
my $newset = POSIX::SigSet->new;
$newset->fillset or die "fillset: $!";
- $newset->delset($_) for @_;
+ for (@_, @UNBLOCKABLE) { $newset->delset($_) or die "delset($_): $!" }
my $oldset = POSIX::SigSet->new;
sig_setmask($newset, $oldset);
$oldset;
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 02/10] t/sigfd: test EVFILT_SIGNAL vs signalfd differences
2023-09-04 10:35 [PATCH 00/10] signal-handling and *BSD fixes Eric Wong
2023-09-04 10:35 ` [PATCH 01/10] ds: don't block important signals we don't use Eric Wong
@ 2023-09-04 10:35 ` Eric Wong
2023-09-04 10:36 ` [PATCH 03/10] t/sigfd: better checks related to SIGWINCH Eric Wong
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2023-09-04 10:35 UTC (permalink / raw)
To: meta
Verify that observed OpenBSD and FreeBSD EVFILT_SIGNAL behavior
works differently than what Linux signalfd does to ease upcoming
changes to PublicInbox::DS.
---
t/sigfd.t | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/t/sigfd.t b/t/sigfd.t
index 0070ca73..15fc818a 100644
--- a/t/sigfd.t
+++ b/t/sigfd.t
@@ -7,6 +7,7 @@ use POSIX qw(:signal_h);
use Errno qw(ENOSYS);
require_ok 'PublicInbox::Sigfd';
use PublicInbox::DS;
+my ($linux_sigfd, $has_sigfd);
SKIP: {
if ($^O ne 'linux' && !eval { require IO::KQueue }) {
@@ -16,16 +17,21 @@ SKIP: {
my $old = PublicInbox::DS::block_signals();
my $hit = {};
my $sig = {};
+ local $SIG{USR2} = sub { $hit->{USR2}->{normal}++ };
local $SIG{HUP} = sub { $hit->{HUP}->{normal}++ };
local $SIG{TERM} = sub { $hit->{TERM}->{normal}++ };
local $SIG{INT} = sub { $hit->{INT}->{normal}++ };
local $SIG{WINCH} = sub { $hit->{WINCH}->{normal}++ };
- for my $s (qw(HUP TERM INT WINCH)) {
+ for my $s (qw(USR2 HUP TERM INT WINCH)) {
$sig->{$s} = sub { $hit->{$s}->{sigfd}++ };
}
+ kill 'USR2', $$ or die "kill $!";
+ ok(!defined($hit->{USR2}), 'no USR2 yet') or diag explain($hit);
PublicInbox::DS->Reset;
my $sigfd = PublicInbox::Sigfd->new($sig, 0);
if ($sigfd) {
+ $linux_sigfd = 1 if $^O eq 'linux';
+ $has_sigfd = 1;
ok($sigfd, 'Sigfd->new works');
kill('HUP', $$) or die "kill $!";
kill('INT', $$) or die "kill $!";
@@ -39,8 +45,14 @@ SKIP: {
for my $s (qw(HUP INT)) {
is($hit->{$s}->{sigfd}, 1, "sigfd fired $s");
is($hit->{$s}->{normal}, undef,
- 'normal $SIG{$s} not fired');
+ "normal \$SIG{$s} not fired");
}
+ SKIP: {
+ skip 'Linux sigfd-only behavior', 1 if !$linux_sigfd;
+ is($hit->{USR2}->{sigfd}, 1,
+ 'USR2 sent before signalfd created received');
+ }
+ ok(!$hit->{USR2}->{normal}, 'USR2 not fired normally');
PublicInbox::DS->Reset;
$sigfd = undef;
@@ -63,7 +75,14 @@ SKIP: {
} else {
skip('signalfd disabled?', 10);
}
- sigprocmask(SIG_SETMASK, $old) or die "sigprocmask $!";
+ 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;
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 03/10] t/sigfd: better checks related to SIGWINCH
2023-09-04 10:35 [PATCH 00/10] signal-handling and *BSD fixes Eric Wong
2023-09-04 10:35 ` [PATCH 01/10] ds: don't block important signals we don't use Eric Wong
2023-09-04 10:35 ` [PATCH 02/10] t/sigfd: test EVFILT_SIGNAL vs signalfd differences Eric Wong
@ 2023-09-04 10:36 ` Eric Wong
2023-09-04 10:36 ` [PATCH 04/10] update devel/syscall-list to devel/sysdefs-list Eric Wong
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2023-09-04 10:36 UTC (permalink / raw)
To: meta
Check to ensure there's a numeric value of SIGWINCH defined for
the given platform. SIGWINCH may also fire while the test is
running due to a user resizing their terminal, so a boolean test
to ensure it fired rather than an exact value check is more
correct.
---
t/sigfd.t | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/sigfd.t b/t/sigfd.t
index 15fc818a..f6449dab 100644
--- a/t/sigfd.t
+++ b/t/sigfd.t
@@ -28,6 +28,7 @@ SKIP: {
kill 'USR2', $$ or die "kill $!";
ok(!defined($hit->{USR2}), 'no USR2 yet') or diag explain($hit);
PublicInbox::DS->Reset;
+ ok($PublicInbox::Syscall::SIGNUM{WINCH}, 'SIGWINCH number defined');
my $sigfd = PublicInbox::Sigfd->new($sig, 0);
if ($sigfd) {
$linux_sigfd = 1 if $^O eq 'linux';
@@ -71,7 +72,7 @@ SKIP: {
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');
- is($hit->{WINCH}->{sigfd}, 1, 'WINCH sigfd fired in event loop');
+ ok($hit->{WINCH}->{sigfd}, 'WINCH sigfd fired in event loop');
} else {
skip('signalfd disabled?', 10);
}
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 04/10] update devel/syscall-list to devel/sysdefs-list
2023-09-04 10:35 [PATCH 00/10] signal-handling and *BSD fixes Eric Wong
` (2 preceding siblings ...)
2023-09-04 10:36 ` [PATCH 03/10] t/sigfd: better checks related to SIGWINCH Eric Wong
@ 2023-09-04 10:36 ` Eric Wong
2023-09-04 10:36 ` [PATCH 05/10] daemon: workaround pre-EVFILT_SIGNAL signals Eric Wong
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2023-09-04 10:36 UTC (permalink / raw)
To: meta
We use it to dump SIGWINCH and _SC_NPROCESSORS_ONLN, so
"sysdefs" is a more appropriate list for *BSD users.
---
MANIFEST | 2 +-
devel/{syscall-list => sysdefs-list} | 47 +++++++++++++++-------------
lib/PublicInbox/Syscall.pm | 7 +++--
3 files changed, 30 insertions(+), 26 deletions(-)
rename devel/{syscall-list => sysdefs-list} (60%)
diff --git a/MANIFEST b/MANIFEST
index 918ec2e1..5964794e 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -123,7 +123,7 @@ contrib/selinux/el7/publicinbox.fc
contrib/selinux/el7/publicinbox.te
devel/README
devel/longest-tests
-devel/syscall-list
+devel/sysdefs-list
examples/README
examples/README.unsubscribe
examples/cgit-commit-filter.lua
diff --git a/devel/syscall-list b/devel/sysdefs-list
similarity index 60%
rename from devel/syscall-list
rename to devel/sysdefs-list
index 0b36c0e2..9764cc29 100755
--- a/devel/syscall-list
+++ b/devel/sysdefs-list
@@ -1,31 +1,37 @@
# Copyright all contributors <meta@public-inbox.org>
# License: AGPL-3.0+ <http://www.gnu.org/licenses/agpl-3.0.txt>
-# Dump syscall numbers under Linux and any other kernel which
-# promises stable syscall numbers. This is to maintain
-# PublicInbox::Syscall
-# DO NOT USE this for *BSDs, none of the current BSD kernels
-# we know about promise stable syscall numbers, we'll use
-# Inline::C to support them.
+# Dump system-specific constant numbers this is to maintain
+# PublicInbox::Syscall and any other system-specific pieces.
+# DO NOT USE syscall numbers for *BSDs, none of the current BSD kernels
+# we know about promise stable syscall numbers (unlike Linux).
+# However, sysconf(3) constants are stable ABI on all safe to dump.
eval 'exec perl -S $0 ${1+"$@"}' # no shebang
if 0; # running under some shell
-use strict;
-use v5.10.1;
+use v5.12;
use File::Temp 0.19;
use POSIX qw(uname);
+use Config;
say '$machine='.(POSIX::uname())[-1];
-my $cc = $ENV{CC} // 'cc';
-my @cflags = split(/\s+/, $ENV{CFLAGS} // '-Wall');
+my $cc = $ENV{CC} // $Config{cc} // 'cc';
+my @cflags = split(/\s+/, $ENV{CFLAGS} // $Config{ccflags} // '-Wall');
my $str = do { local $/; <DATA> };
-my $tmp = File::Temp->newdir('syscall-list-XXXX', TMPDIR => 1);
-my $f = "$tmp/sc.c";
-my $x = "$tmp/sc";
+$str =~ s/^\s*MAYBE\s*(\w+)\s*$/
+#ifdef $1
+ D($1);
+#endif
+/sgxm;
+my $tmp = File::Temp->newdir('sysdefs-list-XXXX', TMPDIR => 1);
+my $f = "$tmp/sysdefs.c";
+my $x = "$tmp/sysdefs";
open my $fh, '>', $f or die "open $f $!";
print $fh $str or die "print $f $!";
close $fh or die "close $f $!";
-system($cc, '-o', $x, $f, @cflags) == 0 or die "cc failed \$?=$?";
+system($cc, '-o', $x, $f, @cflags) == 0 or die "$cc failed \$?=$?";
exec($x);
__DATA__
-#define _GNU_SOURCE
+#ifndef _GNU_SOURCE
+# define _GNU_SOURCE
+#endif
#include <signal.h>
#include <sys/syscall.h>
#include <sys/ioctl.h>
@@ -43,9 +49,7 @@ int main(void)
#ifdef __linux__
D(SYS_epoll_create1);
D(SYS_epoll_ctl);
-#ifdef SYS_epoll_wait
- D(SYS_epoll_wait);
-#endif
+ MAYBE SYS_epoll_wait
D(SYS_epoll_pwait);
D(SYS_signalfd4);
D(SYS_inotify_init1);
@@ -59,13 +63,12 @@ int main(void)
printf("FS_IOC_GETFLAGS=%#lx\nFS_IOC_SETFLAGS=%#lx\n",
(unsigned long)FS_IOC_GETFLAGS, (unsigned long)FS_IOC_SETFLAGS);
#endif
-
-#ifdef SYS_renameat2
- D(SYS_renameat2);
-#endif
+ MAYBE SYS_renameat2
#endif /* Linux, any other OSes with stable syscalls? */
printf("size_t=%zu off_t=%zu pid_t=%zu\n",
sizeof(size_t), sizeof(off_t), sizeof(pid_t));
D(SIGWINCH);
+ MAYBE _SC_NPROCESSORS_ONLN
+
return 0;
}
diff --git a/lib/PublicInbox/Syscall.pm b/lib/PublicInbox/Syscall.pm
index 841a2106..4609b32d 100644
--- a/lib/PublicInbox/Syscall.pm
+++ b/lib/PublicInbox/Syscall.pm
@@ -2,7 +2,7 @@
# specifically the Debian libsys-syscall-perl 0.25-6 version to
# fix upstream regressions in 0.25.
#
-# See devel/syscall-list in the public-inbox source tree for maintenance
+# See devel/sysdefs-list in the public-inbox source tree for maintenance
# <https://80x24.org/public-inbox.git>, and machines from the GCC Farm:
# <https://cfarm.tetaneutral.net/>
#
@@ -246,7 +246,7 @@ if ($^O eq "linux") {
warn <<EOM;
machine=$machine ptrsize=$Config{ptrsize} has no syscall definitions
git clone https://80x24.org/public-inbox.git and
-Send the output of ./devel/syscall-list to meta\@public-inbox.org
+Send the output of ./devel/sysdefs-list to meta\@public-inbox.org
EOM
}
if ($u64_mod_8) {
@@ -259,7 +259,8 @@ EOM
}
# use Inline::C for *BSD-only or general POSIX stuff.
# Linux guarantees stable syscall numbering, BSDs only offer a stable libc
-# use devel/syscall-list on Linux to detect new syscall numbers
+# use devel/sysdefs-list on Linux to detect new syscall numbers and
+# other system constants
############################################################################
# epoll functions
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 05/10] daemon: workaround pre-EVFILT_SIGNAL signals
2023-09-04 10:35 [PATCH 00/10] signal-handling and *BSD fixes Eric Wong
` (3 preceding siblings ...)
2023-09-04 10:36 ` [PATCH 04/10] update devel/syscall-list to devel/sysdefs-list Eric Wong
@ 2023-09-04 10:36 ` Eric Wong
2023-09-04 10:36 ` [PATCH 06/10] watch: ensure children can use signal handlers Eric Wong
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2023-09-04 10:36 UTC (permalink / raw)
To: meta
FreeBSD and OpenBSD kqueue EVFILT_SIGNAL isn't able to handle
blocked signals which were sent before the filter is created.
This behavior differs from Linux signalfd, which can process
blocked signals that were sent before the signalfd existed.
---
lib/PublicInbox/DS.pm | 30 ++++++++++++++++++++++++++----
lib/PublicInbox/Sigfd.pm | 3 ++-
2 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 97546016..5168a6ee 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -24,11 +24,11 @@ use strict;
use v5.10.1;
use parent qw(Exporter);
use bytes qw(length substr); # FIXME(?): needed for PublicInbox::NNTP
-use POSIX qw(WNOHANG sigprocmask SIG_SETMASK);
+use POSIX qw(WNOHANG sigprocmask SIG_SETMASK SIG_UNBLOCK);
use Fcntl qw(SEEK_SET :DEFAULT O_APPEND);
use Time::HiRes qw(clock_gettime CLOCK_MONOTONIC);
use Scalar::Util qw(blessed);
-use PublicInbox::Syscall qw(:epoll);
+use PublicInbox::Syscall qw(:epoll %SIGNUM);
use PublicInbox::Tmpfile;
use Errno qw(EAGAIN EINVAL ECHILD EINTR);
use Carp qw(carp croak);
@@ -259,19 +259,41 @@ sub PostEventLoop () {
: 1
}
+sub allowset ($) {
+ my ($sig) = @_; # { signame => whatever }
+ my $ret = POSIX::SigSet->new;
+ $ret->fillset or die "fillset: $!";
+ for my $s (keys %$sig) {
+ my $num = $SIGNUM{$s} // POSIX->can("SIG$s")->();
+ $ret->delset($num) or die "delset ($s => $num): $!";
+ }
+ for (@UNBLOCKABLE) { $ret->delset($_) or die "delset($_): $!" }
+ $ret;
+}
+
# 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 (;$$) {
my ($sig, $oldset) = @_;
$Epoll //= _InitPoller();
require PublicInbox::Sigfd if $sig;
- my $sigfd = PublicInbox::Sigfd->new($sig, 1) if $sig;
+ my $sigfd = $sig ? PublicInbox::Sigfd->new($sig, 1) : undef;
+ if ($sigfd && $sigfd->{is_kq}) {
+ my $tmp = allowset($sig);
+ local @SIG{keys %$sig} = values(%$sig);
+ sig_setmask($tmp, my $old = POSIX::SigSet->new);
+ # Unlike Linux signalfd, EVFILT_SIGNAL can't handle
+ # signals received before the filter is created,
+ # so we peek at signals here.
+ sig_setmask($old);
+ }
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);
+ sig_setmask($oldset) if $oldset;
+ sigprocmask(SIG_UNBLOCK, allowset($sig)) or die "SIG_UNBLOCK: $!";
PublicInbox::DS->SetLoopTimeout(1000);
}
$_[0] = $sigfd = $sig = undef; # $_[0] == sig
diff --git a/lib/PublicInbox/Sigfd.pm b/lib/PublicInbox/Sigfd.pm
index 3c1d3811..5656baeb 100644
--- a/lib/PublicInbox/Sigfd.pm
+++ b/lib/PublicInbox/Sigfd.pm
@@ -31,8 +31,9 @@ sub new {
$self->SUPER::new($io, EPOLLIN | EPOLLET);
} else { # master main loop
$self->{sock} = $io;
- $self;
}
+ $self->{is_kq} = 1 if tied(*$io);
+ $self;
}
# PublicInbox::Daemon in master main loop (blocking)
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 06/10] watch: ensure children can use signal handlers
2023-09-04 10:35 [PATCH 00/10] signal-handling and *BSD fixes Eric Wong
` (4 preceding siblings ...)
2023-09-04 10:36 ` [PATCH 05/10] daemon: workaround pre-EVFILT_SIGNAL signals Eric Wong
@ 2023-09-04 10:36 ` Eric Wong
2023-09-04 10:36 ` [PATCH 07/10] xap_helper: support SIGTTIN+SIGTTOU worker adjustments Eric Wong
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2023-09-04 10:36 UTC (permalink / raw)
To: meta
Blindly using the signal set inherited from the parent process
is wrong, since the parent (or grandparent) could've blocked all
signals. Ensure children can process signals in the event loop
when sig handlers have to use standard Perl facilities.
---
lib/PublicInbox/Watch.pm | 7 +++----
script/public-inbox-watch | 4 ++--
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index c3b5b791..a2dc125f 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -389,7 +389,7 @@ sub watch_atfork_child ($) {
my $sig = delete $self->{sig};
$sig->{CHLD} = 'DEFAULT';
@SIG{keys %$sig} = values %$sig;
- PublicInbox::DS::sig_setmask($self->{oldset});
+ PublicInbox::DS::sig_setmask(PublicInbox::DS::allowset($sig));
}
sub watch_atfork_parent ($) { _done_for_now($_[0]) }
@@ -533,8 +533,7 @@ sub watch_nntp_init ($$) {
}
sub watch { # main entry point
- my ($self, $sig, $oldset) = @_;
- $self->{oldset} = $oldset;
+ my ($self, $sig) = @_;
my $first_sig;
$self->{sig} //= ($first_sig = $sig);
my $poll = {}; # intvl_seconds => [ uri1, uri2 ]
@@ -546,7 +545,7 @@ sub watch { # main entry point
}
watch_fs_init($self) if $self->{mdre};
local @PublicInbox::DS::post_loop_do = (sub { !$self->quit_done });
- PublicInbox::DS::event_loop($first_sig, $oldset); # calls ->event_step
+ PublicInbox::DS::event_loop($first_sig); # calls ->event_step
_done_for_now($self);
}
diff --git a/script/public-inbox-watch b/script/public-inbox-watch
index 2fb27343..75a9a36b 100755
--- a/script/public-inbox-watch
+++ b/script/public-inbox-watch
@@ -17,7 +17,7 @@ my $do_scan = 1;
GetOptions('scan!' => \$do_scan, # undocumented, testing only
'help|h' => \(my $show_help)) or do { print STDERR $help; exit 1 };
if ($show_help) { print $help; exit 0 };
-my $oldset = PublicInbox::DS::block_signals();
+PublicInbox::DS::block_signals();
STDOUT->autoflush(1);
STDERR->autoflush(1);
local $0 = $0; # local since this script may be eval-ed
@@ -55,5 +55,5 @@ if ($watch) {
# --no-scan is only intended for testing atm, undocumented.
PublicInbox::DS::requeue($scan) if $do_scan;
- $watch->watch($sig, $oldset) while ($watch);
+ $watch->watch($sig) while ($watch);
}
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 07/10] xap_helper: support SIGTTIN+SIGTTOU worker adjustments
2023-09-04 10:35 [PATCH 00/10] signal-handling and *BSD fixes Eric Wong
` (5 preceding siblings ...)
2023-09-04 10:36 ` [PATCH 06/10] watch: ensure children can use signal handlers Eric Wong
@ 2023-09-04 10:36 ` Eric Wong
2023-09-04 10:36 ` [PATCH 08/10] xap_helper.h: include signal.h for sig* functions Eric Wong
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2023-09-04 10:36 UTC (permalink / raw)
To: meta
Being able to tune worker process counts on-the-fly when
xap_helper gets used with -{netd,httpd,imapd} will be useful
for tuning new setups.
---
lib/PublicInbox/IPC.pm | 2 +-
lib/PublicInbox/XapHelper.pm | 103 ++++++++++---
lib/PublicInbox/xap_helper.h | 282 ++++++++++++++++++++++++++++++-----
t/xap_helper.t | 55 +++++--
4 files changed, 371 insertions(+), 71 deletions(-)
diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index 766c377f..528b9133 100644
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -42,7 +42,7 @@ if ($enc && $dec) { # should be custom ops
*ipc_thaw = \&Storable::thaw;
}
-my $recv_cmd = PublicInbox::Spawn->can('recv_cmd4');
+our $recv_cmd = PublicInbox::Spawn->can('recv_cmd4');
our $send_cmd = PublicInbox::Spawn->can('send_cmd4') // do {
require PublicInbox::CmdIPC4;
$recv_cmd //= PublicInbox::CmdIPC4->can('recv_cmd4');
diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index 36266e65..0e79e5e2 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -10,9 +10,11 @@ $GLP->configure(qw(require_order bundling no_ignore_case no_auto_abbrev));
use PublicInbox::Search qw(xap_terms);
use PublicInbox::CodeSearch;
use PublicInbox::IPC;
+use PublicInbox::DS qw(awaitpid);
+use POSIX qw(:signal_h);
use Fcntl qw(LOCK_UN LOCK_EX);
my $X = \%PublicInbox::Search::X;
-our (%SRCH, %PIDS, $parent_pid);
+our (%SRCH, %WORKERS, $parent_pid, $alive, $nworker, $workerset);
our $stderr = \*STDERR;
# only short options for portability in C++ implementation
@@ -171,11 +173,17 @@ sub dispatch {
sub recv_loop {
local $SIG{__WARN__} = sub { print $stderr @_ };
my $rbuf;
- while (!defined($parent_pid) || getppid != $parent_pid) {
- my $req = bless {}, __PACKAGE__;
- my @fds = PublicInbox::IPC::recv_cmd(\*STDIN, $rbuf, 4096*33);
+ my $in = \*STDIN;
+ while (!defined($parent_pid) || getppid == $parent_pid) {
+ PublicInbox::DS::sig_setmask($workerset);
+ my @fds = $PublicInbox::IPC::recv_cmd->($in, $rbuf, 4096*33);
scalar(@fds) or exit(66); # EX_NOINPUT
- $fds[0] // die "recvmsg: $!";
+ if (!defined($fds[0])) {
+ next if $!{EINTR};
+ die "recvmsg: $!";
+ }
+ PublicInbox::DS::block_signals();
+ my $req = bless {}, __PACKAGE__;
my $i = 0;
for my $fd (@fds) {
open($req->{$i++}, '+<&=', $fd) and next;
@@ -195,38 +203,89 @@ sub recv_loop {
}
}
+sub reap_worker { # awaitpid CB
+ my ($pid, $nr) = @_;
+ delete $WORKERS{$nr};
+ if (($? >> 8) == 66) { # EX_NOINPUT
+ $alive = undef;
+ PublicInbox::DS->SetLoopTimeout(1);
+ } elsif ($?) {
+ warn "worker[$nr] died \$?=$?\n";
+ }
+ PublicInbox::DS::requeue(\&start_workers) if $alive;
+}
+
sub start_worker ($) {
my ($nr) = @_;
- my $pid = fork // return warn("fork: $!");
+ my $pid = fork;
+ if (!defined($pid)) {
+ warn("fork: $!");
+ return undef;
+ };
if ($pid == 0) {
- undef %PIDS;
+ undef %WORKERS;
+ PublicInbox::DS::Reset();
+ $SIG{TERM} = sub { $parent_pid = -1 };
+ $SIG{TTIN} = $SIG{TTOU} = 'IGNORE';
+ $SIG{CHLD} = 'DEFAULT'; # Xapian may use this
recv_loop();
exit(0);
} else {
- $PIDS{$pid} = $nr;
+ $WORKERS{$nr} = $pid;
+ awaitpid($pid, \&reap_worker, $nr);
+ }
+}
+
+sub start_workers {
+ for my $nr (grep { !defined($WORKERS{$_}) } (0..($nworker - 1))) {
+ start_worker($nr) if $alive;
}
}
+sub do_sigttou {
+ if ($alive && $nworker > 1) {
+ --$nworker;
+ my @nr = grep { $_ >= $nworker } keys %WORKERS;
+ kill('TERM', @WORKERS{@nr});
+ }
+}
+
+sub xh_alive { $alive || scalar(keys %WORKERS) }
+
sub start (@) {
my (@argv) = @_;
- local (%SRCH, %PIDS, $parent_pid);
+ local (%SRCH, %WORKERS);
+ local $alive = 1;
PublicInbox::Search::load_xapian();
$GLP->getoptionsfromarray(\@argv, my $opt = { j => 1 }, 'j=i') or
die 'bad args';
- return recv_loop() if !$opt->{j};
- die '-j must be >= 0' if $opt->{j} < 0;
- start_worker($_) for (1..($opt->{j}));
-
- my $quit;
- until ($quit) {
- my $p = waitpid(-1, 0) or return;
- if (defined(my $nr = delete $PIDS{$p})) {
- $quit = 1 if ($? >> 8) == 66; # EX_NOINPUT
- start_worker($nr) if !$quit;
- } else {
- warn "W: unknown pid=$p reaped\n";
- }
+ local $workerset = POSIX::SigSet->new;
+ $workerset->fillset or die "fillset: $!";
+ for (@PublicInbox::DS::UNBLOCKABLE) {
+ $workerset->delset($_) or die "delset($_): $!";
}
+
+ local $nworker = $opt->{j};
+ return recv_loop() if $nworker == 0;
+ die '-j must be >= 0' if $nworker < 0;
+ for (POSIX::SIGTERM, POSIX::SIGCHLD) {
+ $workerset->delset($_) or die "delset($_): $!";
+ }
+ local $parent_pid = $$;
+ my $sig = {
+ TTIN => sub {
+ if ($alive) {
+ ++$nworker;
+ PublicInbox::DS::requeue(\&start_workers)
+ }
+ },
+ TTOU => \&do_sigttou,
+ CHLD => \&PublicInbox::DS::enqueue_reap,
+ };
+ PublicInbox::DS::block_signals();
+ start_workers();
+ @PublicInbox::DS::post_loop_do = \&xh_alive;
+ PublicInbox::DS::event_loop($sig);
}
1;
diff --git a/lib/PublicInbox/xap_helper.h b/lib/PublicInbox/xap_helper.h
index 871a381c..493a24f4 100644
--- a/lib/PublicInbox/xap_helper.h
+++ b/lib/PublicInbox/xap_helper.h
@@ -56,15 +56,26 @@
# define STDERR_ASSIGNABLE (0)
#endif
-static const int sock_fd = 0; // SOCK_SEQPACKET as stdin :P
-static pid_t parent_pid;
+// assert functions are used correctly (e.g. ensure hackers don't
+// cause EINVAL/EFAULT). Not for stuff that can fail due to HW
+// failures.
+# define CHECK(type, expect, expr) do { \
+ type ckvar______ = (expr); \
+ assert(ckvar______ == (expect) && "BUG" && __FILE__ && __LINE__); \
+} while (0)
+
+static const int sock_fd = STDIN_FILENO; // SOCK_SEQPACKET as stdin :P
+static volatile pid_t parent_pid; // may be set in worker sighandler (sigw)
+static sigset_t fullset, workerset;
+static bool alive = true;
#if STDERR_ASSIGNABLE
static FILE *orig_err = stderr;
#endif
static int orig_err_fd = -1;
static void *srch_tree; // tsearch + tdelete + twalk
static pid_t *worker_pids; // nr => pid
-static unsigned long nworker;
+static unsigned long nworker, nworker_hwm;
+static int pipefds[2];
// PublicInbox::Search and PublicInbox::CodeSearch generate these:
static void mail_nrp_init(void);
@@ -598,11 +609,21 @@ static bool recv_req(struct req *req, char *rbuf, size_t *len)
msg.msg_control = &cmsg.hdr;
msg.msg_controllen = CMSG_SPACE(RECV_FD_SPACE);
+ // allow SIGTERM to hit
+ CHECK(int, 0, sigprocmask(SIG_SETMASK, &workerset, NULL));
+
ssize_t r = recvmsg(sock_fd, &msg, 0);
- if (r < 0)
- err(EXIT_FAILURE, "recvmsg");
- if (r == 0)
+ if (r == 0) {
exit(EX_NOINPUT); /* grandparent went away */
+ } else if (r < 0) {
+ if (errno == EINTR)
+ return false; // retry recv_loop
+ err(EXIT_FAILURE, "recvmsg");
+ }
+
+ // success! no signals for the rest of the request/response cycle
+ CHECK(int, 0, sigprocmask(SIG_SETMASK, &fullset, NULL));
+
*len = r;
if (r > 0 && cmsg.hdr.cmsg_level == SOL_SOCKET &&
cmsg.hdr.cmsg_type == SCM_RIGHTS) {
@@ -875,9 +896,19 @@ static void stderr_restore(FILE *tmp_err)
clearerr(stderr);
}
+static void sigw(int sig) // SIGTERM handler for worker
+{
+ parent_pid = -1; // break out of recv_loop
+}
+
static void recv_loop(void) // worker process loop
{
static char rbuf[4096 * 33]; // per-process
+ struct sigaction sa = {};
+ sa.sa_handler = sigw;
+
+ CHECK(int, 0, sigaction(SIGTERM, &sa, NULL));
+
while (!parent_pid || getppid() == parent_pid) {
size_t len = sizeof(rbuf);
struct req req = {};
@@ -904,18 +935,6 @@ static void insert_pid(pid_t pid, unsigned nr)
worker_pids[nr] = pid;
}
-static int delete_pid(pid_t pid)
-{
- for (unsigned nr = 0; nr < nworker; nr++) {
- if (worker_pids[nr] == pid) {
- worker_pids[nr] = 0;
- return nr;
- }
- }
- warnx("W: unknown pid=%d reaped", (int)pid);
- return -1;
-}
-
static void start_worker(unsigned nr)
{
pid_t pid = fork();
@@ -925,11 +944,31 @@ static void start_worker(unsigned nr)
insert_pid(pid, nr);
} else {
cleanup_pids();
+ xclose(pipefds[0]);
+ xclose(pipefds[1]);
+ if (signal(SIGCHLD, SIG_DFL) == SIG_ERR)
+ err(EXIT_FAILURE, "signal CHLD");
+ if (signal(SIGTTIN, SIG_IGN) == SIG_ERR)
+ err(EXIT_FAILURE, "signal TTIN");
+ if (signal(SIGTTOU, SIG_IGN) == SIG_ERR)
+ err(EXIT_FAILURE, "signal TTIN");
recv_loop();
exit(0);
}
}
+static void start_workers(void)
+{
+ sigset_t old;
+
+ CHECK(int, 0, sigprocmask(SIG_SETMASK, &fullset, &old));
+ for (unsigned long nr = 0; nr < nworker; nr++) {
+ if (!worker_pids[nr])
+ start_worker(nr);
+ }
+ CHECK(int, 0, sigprocmask(SIG_SETMASK, &old, NULL));
+}
+
static void cleanup_all(void)
{
cleanup_pids();
@@ -939,6 +978,121 @@ static void cleanup_all(void)
#endif
}
+static void sigp(int sig) // parent signal handler
+{
+ static const char eagain[] = "signals coming in too fast";
+ static const char bad_sig[] = "BUG: bad sig\n";
+ static const char write_err[] = "BUG: sigp write: ";
+ char c = 0;
+
+ switch (sig) {
+ case SIGCHLD: c = '.'; break;
+ case SIGTTOU: c = '-'; break;
+ case SIGTTIN: c = '+'; break;
+ default:
+ write(STDERR_FILENO, bad_sig, sizeof(bad_sig) - 1);
+ _exit(EXIT_FAILURE);
+ }
+ ssize_t w = write(pipefds[1], &c, 1);
+ if (w == sizeof(c)) return;
+ int e = 0;
+ if (w < 0) {
+ e = errno;
+ if (e == EAGAIN) {
+ write(STDERR_FILENO, eagain, sizeof(eagain) - 1);
+ return;
+ }
+ }
+ struct iovec iov[3];
+ iov[0].iov_base = (void *)write_err;
+ iov[0].iov_len = sizeof(write_err) - 1;
+ iov[1].iov_base = (void *)(e ? strerror(e) : "zero write");
+ iov[1].iov_len = strlen((const char *)iov[1].iov_base);
+ iov[2].iov_base = (void *)"\n";
+ iov[2].iov_len = 1;
+ (void)writev(STDERR_FILENO, iov, MY_ARRAY_SIZE(iov));
+ _exit(EXIT_FAILURE);
+}
+
+static void reaped_worker(pid_t pid, int st)
+{
+ unsigned long nr = 0;
+ for (; nr < nworker_hwm; nr++) {
+ if (worker_pids[nr] == pid) {
+ worker_pids[nr] = 0;
+ break;
+ }
+ }
+ if (nr >= nworker_hwm) {
+ warnx("W: unknown pid=%d reaped $?=%d", (int)pid, st);
+ return;
+ }
+ if (WIFEXITED(st) && WEXITSTATUS(st) == EX_NOINPUT)
+ alive = false;
+ else if (st)
+ warnx("worker[%lu] died $?=%d", nr, st);
+ if (alive)
+ start_workers();
+}
+
+static void do_sigchld(void)
+{
+ while (1) {
+ int st;
+ pid_t pid = waitpid(-1, &st, WNOHANG);
+ if (pid > 0) {
+ reaped_worker(pid, st);
+ } else if (pid == 0) {
+ return;
+ } else {
+ switch (errno) {
+ case ECHILD: return;
+ case EINTR: break; // can it happen w/ WNOHANG?
+ default: err(EXIT_FAILURE, "BUG: waitpid");
+ }
+ }
+ }
+}
+
+static void do_sigttin(void)
+{
+ if (!alive) return;
+ void *p = reallocarray(worker_pids, nworker + 1, sizeof(pid_t));
+ if (!p) {
+ warn("reallocarray");
+ } else {
+ worker_pids = (pid_t *)p;
+ worker_pids[nworker++] = 0;
+ if (nworker_hwm < nworker)
+ nworker_hwm = nworker;
+ start_workers();
+ }
+}
+
+static void do_sigttou(void)
+{
+ if (!alive || nworker <= 1) return;
+
+ // worker_pids array does not shrink
+ --nworker;
+ for (unsigned long nr = nworker; nr < nworker_hwm; nr++) {
+ pid_t pid = worker_pids[nr];
+ if (pid != 0 && kill(pid, SIGTERM))
+ warn("BUG?: kill(%d, SIGTERM)", (int)pid);
+ }
+}
+
+static size_t living_workers(void)
+{
+ size_t ret = 0;
+
+ for (unsigned long nr = 0; nr < nworker_hwm; nr++) {
+ if (worker_pids[nr])
+ ret++;
+ }
+ return ret;
+}
+
int main(int argc, char *argv[])
{
int c;
@@ -953,7 +1107,7 @@ int main(int argc, char *argv[])
err(EXIT_FAILURE, "dup(2)");
}
- nworker = 0;
+ nworker = 1;
#ifdef _SC_NPROCESSORS_ONLN
long j = sysconf(_SC_NPROCESSORS_ONLN);
if (j > 0)
@@ -981,27 +1135,79 @@ int main(int argc, char *argv[])
errx(EXIT_FAILURE, "BUG: `-%c'", c);
}
}
- if (nworker == 0) {
+ sigset_t pset; // parent-only
+ CHECK(int, 0, sigfillset(&pset));
+
+ // global sigsets:
+ CHECK(int, 0, sigfillset(&fullset));
+ CHECK(int, 0, sigfillset(&workerset));
+
+#define DELSET(sig) do { \
+ CHECK(int, 0, sigdelset(&fullset, sig)); \
+ CHECK(int, 0, sigdelset(&workerset, sig)); \
+ CHECK(int, 0, sigdelset(&pset, sig)); \
+} while (0)
+ DELSET(SIGABRT);
+ DELSET(SIGBUS);
+ DELSET(SIGFPE);
+ DELSET(SIGILL);
+ DELSET(SIGSEGV);
+ DELSET(SIGXCPU);
+ DELSET(SIGXFSZ);
+#undef DELSET
+
+ if (nworker == 0) { // no SIGTERM handling w/o workers
recv_loop();
- } else {
- parent_pid = getpid();
- worker_pids = (pid_t *)calloc(nworker, sizeof(pid_t));
- if (!worker_pids)
- err(EXIT_FAILURE, "calloc");
- for (unsigned i = 0; i < nworker; i++)
- start_worker(i);
-
- int st;
- pid_t pid;
- bool quit = false;
- while ((pid = wait(&st)) > 0) {
- int nr = delete_pid(pid);
- if (nr < 0) continue;
- if (WIFEXITED(st) && WEXITSTATUS(st) == EX_NOINPUT)
- quit = true;
- if (!quit)
- start_worker(nr);
+ return 0;
+ }
+ CHECK(int, 0, sigdelset(&workerset, SIGTERM));
+ CHECK(int, 0, sigdelset(&workerset, SIGCHLD));
+ parent_pid = getpid();
+ nworker_hwm = nworker;
+ worker_pids = (pid_t *)calloc(nworker, sizeof(pid_t));
+ if (!worker_pids) err(EXIT_FAILURE, "calloc");
+
+ if (pipe(pipefds)) err(EXIT_FAILURE, "pipe");
+ int fl = fcntl(F_GETFL, pipefds[1]);
+ if (fl == -1) err(EXIT_FAILURE, "F_GETFL");
+ if (fcntl(F_SETFL, pipefds[1], fl | O_NONBLOCK))
+ err(EXIT_FAILURE, "F_SETFL");
+
+ CHECK(int, 0, sigdelset(&pset, SIGCHLD));
+ CHECK(int, 0, sigdelset(&pset, SIGTTIN));
+ CHECK(int, 0, sigdelset(&pset, SIGTTOU));
+
+ struct sigaction sa = {};
+ sa.sa_handler = sigp;
+
+ CHECK(int, 0, sigaction(SIGTTIN, &sa, NULL));
+ CHECK(int, 0, sigaction(SIGTTOU, &sa, NULL));
+ sa.sa_flags = SA_NOCLDSTOP;
+ CHECK(int, 0, sigaction(SIGCHLD, &sa, NULL));
+
+ CHECK(int, 0, sigprocmask(SIG_SETMASK, &pset, NULL));
+
+ start_workers();
+
+ char sbuf[64];
+ while (alive || living_workers()) {
+ ssize_t n = read(pipefds[0], &sbuf, sizeof(sbuf));
+ if (n < 0) {
+ if (errno == EINTR) continue;
+ err(EXIT_FAILURE, "read");
+ } else if (n == 0) {
+ errx(EXIT_FAILURE, "read EOF");
+ }
+ do_sigchld();
+ for (ssize_t i = 0; i < n; i++) {
+ switch (sbuf[i]) {
+ case '.': break; // do_sigchld already called
+ case '-': do_sigttou(); break;
+ case '+': do_sigttin(); break;
+ default: errx(EXIT_FAILURE, "BUG: c=%c", sbuf[i]);
+ }
}
}
+
return 0;
}
diff --git a/t/xap_helper.t b/t/xap_helper.t
index b68f2773..f4b3581f 100644
--- a/t/xap_helper.t
+++ b/t/xap_helper.t
@@ -95,21 +95,55 @@ my $test = sub {
my $stats = do { local $/; <$err_rd> };
is($stats, "mset.size=6 nr_out=6\n", 'mset.size reported');
- if ($arg[-1] !~ /\('-j0'\)/) {
- kill('KILL', $cinfo{pid});
+ return $ar if $cinfo{pid} == $pid;
+
+ # test worker management:
+ kill('TERM', $cinfo{pid});
+ my $tries = 0;
+ do {
$r = $doreq->($s, qw(test_inspect -d), $ibx_idx[0]);
- %info = map {
- split(/=/, $_, 2)
- } split(/ /, do { local $/; <$r> });
- isnt($info{pid}, $cinfo{pid}, 'spawned new worker');
+ %info = map { split(/=/, $_, 2) }
+ split(/ /, do { local $/; <$r> });
+ } while ($info{pid} == $cinfo{pid} && ++$tries < 10);
+ isnt($info{pid}, $cinfo{pid}, 'spawned new worker');
+
+ my %pids;
+ $tries = 0;
+ my @ins = ($s, qw(test_inspect -d), $ibx_idx[0]);
+ kill('TTIN', $pid);
+ until (scalar(keys %pids) >= 2 || ++$tries > 10) {
+ tick;
+ my @r = map { $doreq->(@ins) } (0..5);
+ for my $fh (@r) {
+ my $buf = do { local $/; <$fh> } // die "read: $!";
+ $buf =~ /\bpid=(\d+)/ and $pids{$1} = undef;
+ }
+ }
+ is(scalar keys %pids, 2, 'have two pids');
+
+ kill('TTOU', $pid);
+ %pids = ();
+ my $delay = $tries * 0.11 * ($ENV{VALGRIND} ? 10 : 1);
+ $tries = 0;
+ diag 'waiting '.$delay.'s for SIGTTOU';
+ tick($delay);
+ until (scalar(keys %pids) == 1 || ++$tries > 100) {
+ %pids = ();
+ my @r = map { $doreq->(@ins) } (0..5);
+ for my $fh (@r) {
+ my $buf = do { local $/; <$fh> } // die "read: $!";
+ $buf =~ /\bpid=(\d+)/ and $pids{$1} = undef;
+ }
}
+ is(scalar keys %pids, 1, 'have one pid') or diag explain(\%pids);
+ is($info{pid}, (keys %pids)[0], 'kept oldest PID after TTOU');
+
$ar;
};
-my $ar;
my @NO_CXX = (1);
unless ($ENV{TEST_XH_CXX_ONLY}) {
- $ar = $test->(qw[-MPublicInbox::XapHelper -e
+ my $ar = $test->(qw[-MPublicInbox::XapHelper -e
PublicInbox::XapHelper::start('-j0')]);
$ar = $test->(qw[-MPublicInbox::XapHelper -e
PublicInbox::XapHelper::start('-j1')]);
@@ -119,10 +153,10 @@ SKIP: {
require PublicInbox::XapHelperCxx;
PublicInbox::XapHelperCxx::check_build();
};
- skip "XapHelperCxx build: $@", 1 if $@;
+ skip "XapHelperCxx build: $@", 1 if $@ || $ENV{PI_NO_CXX};
@NO_CXX = $ENV{TEST_XH_CXX_ONLY} ? (0) : (0, 1);
- $ar = $test->(qw[-MPublicInbox::XapHelperCxx -e
+ my $ar = $test->(qw[-MPublicInbox::XapHelperCxx -e
PublicInbox::XapHelperCxx::start('-j0')]);
$ar = $test->(qw[-MPublicInbox::XapHelperCxx -e
PublicInbox::XapHelperCxx::start('-j1')]);
@@ -142,6 +176,7 @@ my @id2root;
close $fh;
}
+my $ar;
for my $n (@NO_CXX) {
local $ENV{PI_NO_CXX} = $n;
my ($xhc, $pid) = PublicInbox::XapClient::start_helper('-j0');
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 08/10] xap_helper.h: include signal.h for sig* functions
2023-09-04 10:35 [PATCH 00/10] signal-handling and *BSD fixes Eric Wong
` (6 preceding siblings ...)
2023-09-04 10:36 ` [PATCH 07/10] xap_helper: support SIGTTIN+SIGTTOU worker adjustments Eric Wong
@ 2023-09-04 10:36 ` Eric Wong
2023-09-04 10:36 ` [PATCH 09/10] tests: add `+SCM_RIGHTS' as a require_mods target Eric Wong
2023-09-04 10:36 ` [PATCH 10/10] test_common: start_script: set default signals Eric Wong
9 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2023-09-04 10:36 UTC (permalink / raw)
To: meta
This is documented by all sig* manpages of FreeBSD and Linux,
but only OpenBSD fails to build without this header.
---
lib/PublicInbox/xap_helper.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/PublicInbox/xap_helper.h b/lib/PublicInbox/xap_helper.h
index 493a24f4..0f16d316 100644
--- a/lib/PublicInbox/xap_helper.h
+++ b/lib/PublicInbox/xap_helper.h
@@ -30,6 +30,7 @@
#include <fcntl.h>
#include <limits.h>
#include <search.h>
+#include <signal.h>
#include <stdio.h>
#include <string.h>
#include <sysexits.h>
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 09/10] tests: add `+SCM_RIGHTS' as a require_mods target
2023-09-04 10:35 [PATCH 00/10] signal-handling and *BSD fixes Eric Wong
` (7 preceding siblings ...)
2023-09-04 10:36 ` [PATCH 08/10] xap_helper.h: include signal.h for sig* functions Eric Wong
@ 2023-09-04 10:36 ` Eric Wong
2023-09-04 10:36 ` [PATCH 10/10] test_common: start_script: set default signals Eric Wong
9 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2023-09-04 10:36 UTC (permalink / raw)
To: meta
We'll also ensure the existing `lei' target expands to depend on
`+SCM_RIGHTS', and use require_mods in t/lei-import-nntp.t and
t/lei.t so they can be skipped when Inline::C and Socket::MsgHdr
are missing on OpenBSD.
---
lib/PublicInbox/TestCommon.pm | 15 +++++++++------
t/lei-import-nntp.t | 4 ++--
t/lei.t | 3 ++-
t/xap_helper.t | 4 +---
4 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 6da993af..a90e89d1 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -134,7 +134,7 @@ sub require_mods {
while (my $mod = shift(@mods)) {
if ($mod eq 'lei') {
require_git(2.6, $maybe ? $maybe : ());
- push @mods, qw(DBD::SQLite Search::Xapian);
+ push @mods, qw(DBD::SQLite Search::Xapian +SCM_RIGHTS);
$mod = 'json'; # fall-through
}
if ($mod eq 'json') {
@@ -155,6 +155,11 @@ sub require_mods {
PublicInbox::Search::load_xapian()) {
next;
}
+ } elsif ($mod eq '+SCM_RIGHTS') {
+ if (my $msg = need_scm_rights()) {
+ push @need, $msg;
+ next;
+ }
} elsif (index($mod, '||') >= 0) { # "Foo||Bar"
my $ok;
for my $m (split(/\Q||\E/, $mod)) {
@@ -567,13 +572,13 @@ sub ignore_inline_c_missing {
grep(!/\bInline\b/, split(/^/m, $_[0])))));
}
-sub no_scm_rights () {
+sub need_scm_rights () {
state $ok = PublicInbox::Spawn->can('send_cmd4') || do {
require PublicInbox::Syscall;
- PublicInbox::Syscall->can('send_cmd4');
+ PublicInbox::Syscall->can('send_cmd4'); # Linux only
} || eval { require Socket::MsgHdr; 1 };
return if $ok;
- 'Inline::C unconfigured/missing '.
+ 'need SCM_RIGHTS support: Inline::C unconfigured/missing '.
'(mkdir -p ~/.cache/public-inbox/inline-c) OR Socket::MsgHdr missing';
}
@@ -601,8 +606,6 @@ SKIP: {
$ENV{LANG} = $ENV{LC_ALL} = 'C';
my (undef, $fn, $lineno) = caller(0);
my $t = "$fn:$lineno";
- state $msg = no_scm_rights();
- skip $msg, 1 if $msg;
$lei_opt = { 1 => \$lei_out, 2 => \$lei_err };
my ($daemon_pid, $for_destroy, $daemon_xrd);
my $tmpdir = $test_opt->{tmpdir};
diff --git a/t/lei-import-nntp.t b/t/lei-import-nntp.t
index 2c48d973..c3ee06a2 100644
--- a/t/lei-import-nntp.t
+++ b/t/lei-import-nntp.t
@@ -1,9 +1,9 @@
#!perl -w
-# Copyright (C) 2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
use strict; use v5.10.1; use PublicInbox::TestCommon;
require_git 2.6;
-require_mods(qw(json DBD::SQLite Search::Xapian Net::NNTP));
+require_mods(qw(lei json DBD::SQLite Search::Xapian Net::NNTP));
my ($ro_home, $cfg_path) = setup_public_inboxes;
my ($tmpdir, $for_destroy) = tmpdir;
my $sock = tcp_server;
diff --git a/t/lei.t b/t/lei.t
index 5d0fa622..d83bde69 100644
--- a/t/lei.t
+++ b/t/lei.t
@@ -1,7 +1,8 @@
#!perl -w
-# Copyright (C) 2020-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
use strict; use v5.10.1; use PublicInbox::TestCommon;
+require_mods 'lei';
use File::Path qw(rmtree);
# this only tests the basic help/config/init/completion bits of lei;
diff --git a/t/xap_helper.t b/t/xap_helper.t
index f4b3581f..0a211329 100644
--- a/t/xap_helper.t
+++ b/t/xap_helper.t
@@ -3,9 +3,7 @@
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
use v5.12;
use PublicInbox::TestCommon;
-require_mods(qw(DBD::SQLite Search::Xapian));
-my $msg = no_scm_rights;
-plan(skip_all => $msg) if $msg; # TODO: FIFO support?
+require_mods(qw(DBD::SQLite Search::Xapian +SCM_RIGHTS)); # TODO: FIFO support?
use PublicInbox::Spawn qw(spawn);
use Socket qw(AF_UNIX SOCK_SEQPACKET SOCK_STREAM);
require PublicInbox::AutoReap;
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 10/10] test_common: start_script: set default signals
2023-09-04 10:35 [PATCH 00/10] signal-handling and *BSD fixes Eric Wong
` (8 preceding siblings ...)
2023-09-04 10:36 ` [PATCH 09/10] tests: add `+SCM_RIGHTS' as a require_mods target Eric Wong
@ 2023-09-04 10:36 ` Eric Wong
9 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2023-09-04 10:36 UTC (permalink / raw)
To: meta
We need to ensure signal handlers in the child process aren't
inherited from the parent. This change was originally intended
to block signals all the way until PublicInbox::Daemon and
PublicInbox::Watch were fully ready to handle them (preferably
via EVFILT_SIGNAL or signalfd); but that proved unrealistic.
Now, all signal handlers are restored to their default values
before signals are unblocked.
Drop a redundant DS->Reset while we're at it.
---
lib/PublicInbox/TestCommon.pm | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index a90e89d1..b2774f58 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -478,7 +478,11 @@ sub start_script {
}
$tail = tail_f(@paths);
}
- my $pid = fork // die "fork: $!\n";
+ my $oset = PublicInbox::DS::block_signals();
+ require PublicInbox::OnDestroy;
+ my $tmp_mask = PublicInbox::OnDestroy->new(
+ \&PublicInbox::DS::sig_setmask, $oset);
+ my $pid = fork // die "fork: $!";
if ($pid == 0) {
eval { PublicInbox::DS->Reset };
for (@{delete($opt->{-CLOFORK}) // []}) {
@@ -504,8 +508,10 @@ sub start_script {
}
if ($opt->{-C}) { chdir($opt->{-C}) or die "chdir: $!" }
$0 = join(' ', @$cmd);
+ local @SIG{keys %SIG} = map { undef } values %SIG;
+ local $SIG{FPE} = 'IGNORE'; # Perl default
+ undef $tmp_mask;
if ($sub) {
- eval { PublicInbox::DS->Reset };
_run_sub($sub, $key, \@argv);
POSIX::_exit($? >> 8);
} else {
@@ -513,6 +519,7 @@ sub start_script {
die "FAIL: ",join(' ', $key, @argv), ": $!\n";
}
}
+ undef $tmp_mask;
require PublicInbox::AutoReap;
my $td = PublicInbox::AutoReap->new($pid);
$td->{-extra} = $tail;
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-09-04 10:36 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-04 10:35 [PATCH 00/10] signal-handling and *BSD fixes Eric Wong
2023-09-04 10:35 ` [PATCH 01/10] ds: don't block important signals we don't use Eric Wong
2023-09-04 10:35 ` [PATCH 02/10] t/sigfd: test EVFILT_SIGNAL vs signalfd differences Eric Wong
2023-09-04 10:36 ` [PATCH 03/10] t/sigfd: better checks related to SIGWINCH Eric Wong
2023-09-04 10:36 ` [PATCH 04/10] update devel/syscall-list to devel/sysdefs-list Eric Wong
2023-09-04 10:36 ` [PATCH 05/10] daemon: workaround pre-EVFILT_SIGNAL signals Eric Wong
2023-09-04 10:36 ` [PATCH 06/10] watch: ensure children can use signal handlers Eric Wong
2023-09-04 10:36 ` [PATCH 07/10] xap_helper: support SIGTTIN+SIGTTOU worker adjustments Eric Wong
2023-09-04 10:36 ` [PATCH 08/10] xap_helper.h: include signal.h for sig* functions Eric Wong
2023-09-04 10:36 ` [PATCH 09/10] tests: add `+SCM_RIGHTS' as a require_mods target Eric Wong
2023-09-04 10:36 ` [PATCH 10/10] test_common: start_script: set default signals Eric Wong
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).