Date: Sat, 9 Dec 2017 13:02:03 -0800
From: Noah Misch <noah@leadboat.com>
To: pgsql-hackers@postgresql.org
Subject: SIGPIPE in TAP tests
Message-ID: <20171209210203.GC3362632@rfd.leadboat.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="HlL+5n6rz5pIUxbD"
Content-Disposition: inline
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


--HlL+5n6rz5pIUxbD
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Two buildfarm runs[1][2] from the last 90 days have failed in
src/test/authentication, like this:

  t/001_password.pl .. 
  Failed 3/8 subtests 
  t/002_saslprep.pl .. ok     1815 ms ( 0.00 usr  0.00 sys +  0.89 cusr  0.26 csys =  1.15 CPU)

  Test Summary Report
  -------------------
  t/001_password.pl (Wstat: 13 Tests: 5 Failed: 0)
    Non-zero wait status: 13
    Parse errors: Bad plan.  You planned 8 tests but ran 5.
  Files=2, Tests=17,  3 wallclock secs ( 0.04 usr  0.00 sys +  1.67 cusr  0.50 csys =  2.21 CPU)
  Result: FAIL

Compared to a good run, the other logs just end suddenly after the expected
"FATAL:  password authentication failed".  "Wstat: 13" means the Perl process
died to signal 13 (SIGPIPE).  This test invokes psql in a way that fails
authentication, and it writes "SELECT 1" to psql's stdin.  The SIGPIPE happens
if the psql process exits before that write.  I can reproduce this
consistently by patching a delay into IPC::Run:

--- IPC/Run.pm~	2017-05-12 06:46:43.000000000 -0700
+++ IPC/Run.pm	2017-12-09 00:24:06.876132628 -0800
@@ -1449,6 +1449,8 @@
 sub run {
     local $in_run = 1;     ## Allow run()-only optimizations.
     my IPC::Run $self = start(@_);
+    use Time::HiRes 'usleep';
+    usleep 100_000;

The two src/test/authentication tests then fail, but nothing else fails.
Let's ignore SIGPIPE in all TAP tests, which leaves some evidence in
regress_log_001_password:

  ack Broken pipe: write( 13, 'SELECT 1' ) at /home/nm/sw/cpan/lib/perl5/IPC/Run/IO.pm line 549.

To fix the actual failures, we can cease sending "SELECT 1"; it's enough to
disconnect immediately.  Patch attached.

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2017-11-23%2008%3A37%3A17
[2] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mandrill&dt=2017-10-30%2000%3A14%3A09

--HlL+5n6rz5pIUxbD
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="sigpipe-tap-v1.patch"

diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index 2d3f674..9340f2f 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -44,8 +44,7 @@ sub test_role
 
 	$status_string = 'success' if ($expected_res eq 0);
 
-	my $res =
-	  $node->psql('postgres', 'SELECT 1', extra_params => [ '-U', $role ]);
+	my $res = $node->psql('postgres', undef, extra_params => [ '-U', $role ]);
 	is($res, $expected_res,
 		"authentication $status_string for method $method, role $role");
 }
diff --git a/src/test/authentication/t/002_saslprep.pl b/src/test/authentication/t/002_saslprep.pl
index df9f85d..e09273e 100644
--- a/src/test/authentication/t/002_saslprep.pl
+++ b/src/test/authentication/t/002_saslprep.pl
@@ -41,8 +41,7 @@ sub test_login
 	$status_string = 'success' if ($expected_res eq 0);
 
 	$ENV{"PGPASSWORD"} = $password;
-	my $res =
-	  $node->psql('postgres', 'SELECT 1', extra_params => [ '-U', $role ]);
+	my $res = $node->psql('postgres', undef, extra_params => [ '-U', $role ]);
 	is($res, $expected_res,
 		"authentication $status_string for role $role with password $password"
 	);
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index d1a2eb5..6019040 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -75,6 +75,10 @@ BEGIN
 INIT
 {
 
+	# Return EPIPE instead of killing the process with SIGPIPE.  An affected
+	# test may still fail, but it's more likely to report useful facts.
+	$SIG{PIPE} = 'IGNORE';
+
 	# Determine output directories, and create them.  The base path is the
 	# TESTDIR environment variable, which is normally set by the invoking
 	# Makefile.

--HlL+5n6rz5pIUxbD--