* [PATCH 1/2] t/httpd-corner: improve reliability and diagnostics
2020-04-16 1:48 [PATCH 0/2] more test fixes Eric Wong
@ 2020-04-16 1:48 ` Eric Wong
2020-04-16 1:48 ` [PATCH 2/2] t/httpd-unix: reliability for non-signalfd/EVFILT_SIGNAL Eric Wong
1 sibling, 0 replies; 4+ messages in thread
From: Eric Wong @ 2020-04-16 1:48 UTC (permalink / raw)
To: meta
The graceful-shutdown-on-PUT test is unreliable because we can't
rely on a FIFO as we do with the GET tests. So increase the
delay to 100ms since that seems enough on my system even with
CONFIG_HZ=100.
Add a timeout and backtrace to the $check_self sub to help with
further diagnostics while we're at it, too.
It would be nice if there were a portable syscall tracing
mechanism we could attach to the -httpd process to make the test
more determistic...
---
t/httpd-corner.t | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/t/httpd-corner.t b/t/httpd-corner.t
index 21b5c560..f25a9a9c 100644
--- a/t/httpd-corner.t
+++ b/t/httpd-corner.t
@@ -16,6 +16,7 @@ use IO::Socket::UNIX;
use Fcntl qw(:seek);
use Socket qw(IPPROTO_TCP TCP_NODELAY SOL_SOCKET);
use POSIX qw(mkfifo);
+use Carp ();
my ($tmpdir, $for_destroy) = tmpdir();
my $fifo = "$tmpdir/fifo";
ok(defined mkfifo($fifo, 0777), 'created FIFO');
@@ -298,6 +299,8 @@ my $len = length $str;
is($len, 26, 'got the alphabet');
my $check_self = sub {
my ($conn) = @_;
+ vec(my $rbits, fileno($conn), 1) = 1;
+ select($rbits, undef, undef, 30) or Carp::confess('timed out');
$conn->read(my $buf, 4096);
my ($head, $body) = split(/\r\n\r\n/, $buf, 2);
like($head, qr/\r\nContent-Length: 40\r\n/s, 'got expected length');
@@ -391,17 +394,20 @@ SKIP: {
{
my $conn = conn_for($sock, 'graceful termination during slow request');
- $conn->write("PUT /sha1 HTTP/1.0\r\n");
- delay();
- $conn->write("Content-Length: $len\r\n");
- delay();
- $conn->write("\r\n");
- is($td->kill, 1, 'started graceful shutdown');
- delay();
+ $conn->write("PUT /sha1 HTTP/1.0\r\nContent-Length: $len\r\n\r\n");
+
+ # XXX ugh, want a reliable and non-intrusive way to detect
+ # that the server has started buffering our partial request so we
+ # can reliably test graceful termination. Maybe making this and
+ # similar tests dependent on Linux strace is a possibility?
+ delay(0.1);
+
+ is($td->kill, 1, 'start graceful shutdown');
my $n = 0;
foreach my $c ('a'..'z') {
$n += $conn->write($c);
}
+ ok(kill(0, $td->{pid}), 'graceful shutdown did not kill httpd');
is($n, $len, 'wrote alphabet');
$check_self->($conn);
$td->join;
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] t/httpd-unix: reliability for non-signalfd/EVFILT_SIGNAL
2020-04-16 1:48 [PATCH 0/2] more test fixes Eric Wong
2020-04-16 1:48 ` [PATCH 1/2] t/httpd-corner: improve reliability and diagnostics Eric Wong
@ 2020-04-16 1:48 ` Eric Wong
2020-04-17 5:22 ` [PATCHv2 2/2] t/httpd-unix: skip some tests w/o signalfd|EVFILT_SIGNAL Eric Wong
1 sibling, 1 reply; 4+ messages in thread
From: Eric Wong @ 2020-04-16 1:48 UTC (permalink / raw)
To: meta
Perl seems unable to detect a pending signal (`PL_sig_pending' flag)
even in a sleep(1) loop which wakes up every second.
The (SIGTTOU|SIGUSR2|SIGQUIT) signal itself in the
`PL_psig_pend[sig]' array is not lost, so we can send SIGCHLD
to the process to force the `PL_sig_pending' into being set,
again.
I tested this commit in a loop with the following patch on a
Debian GNU/Linux system to disable signalfd use:
diff --git a/lib/PublicInbox/Sigfd.pm b/lib/PublicInbox/Sigfd.pm
index f500902e..597b40d1 100644
--- a/lib/PublicInbox/Sigfd.pm
+++ b/lib/PublicInbox/Sigfd.pm
@@ -12,6 +12,7 @@ use IO::Handle ();
# are available.
sub new {
my ($class, $sig, $flags) = @_;
+ return;
my $self = fields::new($class);
my %signo = map {;
my $cb = $sig->{$_};
---
t/httpd-unix.t | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/t/httpd-unix.t b/t/httpd-unix.t
index 7ebc3464..3bfded0c 100644
--- a/t/httpd-unix.t
+++ b/t/httpd-unix.t
@@ -81,13 +81,27 @@ check_sock($unix);
ok(-S $unix, 'unix socket still exists');
}
+# Perl can delay signal dispatches due to races, so we repeatedly
+# poke a process with an innocuous signal (SIGCHLD) when signalfd(2)
+# (or EVFILT_SIGNAL) is missing
+my $sig_poke;
+if (my $sigfd = PublicInbox::Sigfd->new({}, 0)) {
+ $sig_poke = sub {}; # noop, we've got signalfd or EVFILT_SIGNAL
+} else {
+ $sig_poke = sub {
+ my $pid = shift;
+ kill('CHLD', $pid);
+ };
+}
+
sub delay_until {
my $cond = shift;
- for (1..1000) {
+ my $end = time + 30;
+ do {
return if $cond->();
select undef, undef, undef, 0.012;
- }
- Carp::croak('condition failed');
+ } until (time > $end);
+ Carp::confess('condition failed');
}
SKIP: {
@@ -133,19 +147,23 @@ SKIP: {
kill('USR2', $pid) or die "USR2 failed: $!";
delay_until(sub {
+ $sig_poke->($pid);
$pid != (eval { $read_pid->($pid_file) } // $pid)
});
my $new_pid = $read_pid->($pid_file);
isnt($new_pid, $pid, 'new child started');
+ ok($new_pid > 0, '$new_pid valid');
delay_until(sub { -s "$pid_file.oldbin" });
my $old_pid = $read_pid->("$pid_file.oldbin");
is($old_pid, $pid, '.oldbin pid file written');
+ ok($old_pid > 0, '$old_pid valid');
check_sock($unix); # ensures $new_pid is ready to receive signals
# first, back out of the upgrade
kill('QUIT', $new_pid) or die "kill new PID failed: $!";
delay_until(sub {
+ $sig_poke->($new_pid);
$pid == (eval { $read_pid->($pid_file) } // 0)
});
is($read_pid->($pid_file), $pid, 'old PID file restored');
^ permalink raw reply related [flat|nested] 4+ messages in thread