[PATCH 0/2] more test fixes

[PATCH 0/2] more test fixes

[Eric Wong]

Still testing these, but I think these will be the last two
patches before 1.4.0.

Eric Wong (2):
  t/httpd-corner: improve reliability and diagnostics
  t/httpd-unix: reliability for non-signalfd/EVFILT_SIGNAL

 t/httpd-corner.t | 20 +++++++++++++-------
 t/httpd-unix.t   | 24 +++++++++++++++++++++---
 2 files changed, 34 insertions(+), 10 deletions(-)

[PATCH 1/2] t/httpd-corner: improve reliability and diagnostics

[Eric Wong]

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;

[PATCH 2/2] t/httpd-unix: reliability for non-signalfd/EVFILT_SIGNAL

[Eric Wong]

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');

[PATCHv2 2/2] t/httpd-unix: skip some tests w/o signalfd|EVFILT_SIGNAL

[Eric Wong]

After more testing on a FreeBSD 11.2 VM, I'm dropping
https://public-inbox.org/meta/20200416014838.5939-3-e@yhbt.net/
https://public-inbox.org/meta/20200416014838.5939-3-e@yhbt.net/
in favor of the following patch:

-------8<-------
Subject: [PATCHv2 2/2] t/httpd-unix: skip some tests w/o signalfd|EVFILT_SIGNAL

Some of these tests just don't seem reliable enough with the
way Perl does portable signal handling.
---
 t/httpd-unix.t | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/t/httpd-unix.t b/t/httpd-unix.t
index 7ebc3464..363f3648 100644
--- a/t/httpd-unix.t
+++ b/t/httpd-unix.t
@@ -81,17 +81,23 @@ check_sock($unix);
 	ok(-S $unix, 'unix socket still exists');
 }
 
+# portable Perl can delay or miss signal dispatches due to races,
+# so disable some tests on systems lacking signalfd(2) or EVFILT_SIGNAL
+my $has_sigfd = PublicInbox::Sigfd->new({}, 0) ? 1 : $ENV{TEST_UNRELIABLE};
+
 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: {
-	require_mods('Net::Server::Daemonize', 20);
+	require_mods('Net::Server::Daemonize', 52);
+	$has_sigfd or skip('signalfd / EVFILT_SIGNAL not available', 52);
 	my $pid_file = "$tmpdir/pid";
 	my $read_pid = sub {
 		my $f = shift;
@@ -137,9 +143,11 @@ SKIP: {
 	});
 	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