From fab84c7787f25756a9d7bcb8bc89145d237e8e85 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 24 Apr 2016 15:31:36 -0400
Subject: [PATCH] Improve PostgresNode.pm's logic for detecting already-in-use
 ports.

Buildfarm members bowerbird and jacana have shown intermittent "could not
bind IPv4 socket" failures in the BinInstallCheck stage since mid-December,
shortly after commits 1caef31d9e550408 and 9821492ee417a591 changed the
logic for selecting which port to use in temporary installations.  One
plausible explanation is that we are randomly selecting ports that are
already in use for some non-Postgres purpose.  Although the code tried
to defend against already-in-use ports, it used pg_isready to probe
the port which is quite unhelpful: if some non-Postgres server responds
at the given address, pg_isready will generally say "no response",
leading to exactly the wrong conclusion about whether the port is free.

Instead, let's use a simple TCP connect() call to see if anything answers
without making assumptions about what it is.  Note that this means there's
no direct check for a conflicting Unix socket, but that should be okay
because there should be no other Unix sockets in use in the temporary
socket directory created for a test run.

This is only a partial solution for the TCP case, since if the port number
is in use for an outgoing connection rather than a listening socket, we'll
fail to detect that.  We could try to bind() to the proposed port as a
means of detecting that case, but that would introduce its own failure
modes, since the system might consider the address to remain reserved for
some period of time after we drop the bound socket.  Close study of the
errors returned by bowerbird and jacana suggests that what we're seeing
there may be conflicts with listening not outgoing sockets, so let's try
this and see if it improves matters.  It's certainly better than what's
there now, in any case.

Michael Paquier, adjusted by me to work on non-Windows as well as Windows
---
 src/test/perl/PostgresNode.pm | 42 +++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index e42c1174cb..cd2e974de1 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -90,6 +90,7 @@ use File::Spec;
 use File::Temp ();
 use IPC::Run;
 use RecursiveCopy;
+use Socket;
 use Test::More;
 use TestLib ();
 use Scalar::Util qw(blessed);
@@ -98,14 +99,15 @@ our @EXPORT = qw(
   get_new_node
 );
 
-our ($test_pghost, $last_port_assigned, @all_nodes);
+our ($test_localhost, $test_pghost, $last_port_assigned, @all_nodes);
 
 INIT
 {
 	# PGHOST is set once and for all through a single series of tests when
 	# this module is loaded.
+	$test_localhost = "127.0.0.1";
 	$test_pghost =
-	  $TestLib::windows_os ? "127.0.0.1" : TestLib::tempdir_short;
+	  $TestLib::windows_os ? $test_localhost : TestLib::tempdir_short;
 	$ENV{PGHOST}     = $test_pghost;
 	$ENV{PGDATABASE} = 'postgres';
 
@@ -347,7 +349,7 @@ sub set_replication_conf
 	else
 	{
 		print $hba
-"host replication all 127.0.0.1/32 sspi include_realm=1 map=regress\n";
+"host replication all $test_localhost/32 sspi include_realm=1 map=regress\n";
 	}
 	close $hba;
 }
@@ -839,19 +841,31 @@ sub get_new_node
 
 	while ($found == 0)
 	{
-		# wrap correctly around range end
+		# advance $port, wrapping correctly around range end
 		$port = 49152 if ++$port >= 65536;
-		print "# Checking for port $port\n";
-		if (!TestLib::run_log([ 'pg_isready', '-p', $port ]))
-		{
-			$found = 1;
+		print "# Checking port $port\n";
 
-			# Found a potential candidate port number.  Check first that it is
-			# not included in the list of registered nodes.
-			foreach my $node (@all_nodes)
-			{
-				$found = 0 if ($node->port == $port);
-			}
+		# Check first that candidate port number is not included in
+		# the list of already-registered nodes.
+		$found = 1;
+		foreach my $node (@all_nodes)
+		{
+			$found = 0 if ($node->port == $port);
+		}
+
+		# Check to see if anything else is listening on this TCP port.
+		# This is *necessary* on Windows, and seems like a good idea
+		# on Unixen as well, even though we don't ask the postmaster
+		# to open a TCP port on Unix.
+		if ($found == 1)
+		{
+			my $iaddr   = inet_aton($test_localhost);
+			my $paddr   = sockaddr_in($port, $iaddr);
+			my $proto   = getprotobyname("tcp");
+
+			socket(SOCK, PF_INET, SOCK_STREAM, $proto) or die;
+			$found = 0 if connect(SOCK, $paddr);
+			close(SOCK);
 		}
 	}