On Tue, Aug 16, 2022 at 4:14 PM Thomas Munro <thomas.mu...@gmail.com> wrote:
> On Tue, Aug 16, 2022 at 1:16 PM Andres Freund <and...@anarazel.de> wrote:
> > > But let's suppose we want to play by a timid interpretation of that page's
> > > "do not issue low-level or STDIO.H I/O routines".  It also says that 
> > > SIGINT
> > > is special and runs the handler in a new thread (in a big warning box
> > > because that has other hazards that would break other kinds of code).  
> > > Well,
> > > we *know* it's safe to unlink files in another thread... so... how cheesy
> > > would it be if we just did raise(SIGINT) in the real handlers?
> >
> > Not quite sure I understand. You're proposing to raise(SIGINT) for all other
> > handlers, so that signal_remove_temp() gets called in another thread, 
> > because
> > we assume that'd be safe because doing file IO in other threads is safe? 
> > That
> > assumes the signal handler invocation infrastructure isn't the problem...
>
> That's what I was thinking about, yeah.  But after some more reading,
> now I'm wondering if we'd even need to do that, or what I'm missing.
> The 6 listed signals in the manual are SIGABRT, SIGFPE, SIGILL,
> SIGINT, SIGSEGV and SIGTERM (the 6 required by C).  We want to run
> signal_remove_temp() on SIGHUP (doesn't exist, we made it up), SIGINT,
> SIGPIPE (doesn't exist, we made it up), and SIGTERM (exists for C spec
> compliance but will never be raised by the system according to the
> manual, and we don't raise it ourselves IIUC).  So the only case we
> actually have to consider is SIGINT, and SIGINT handlers run in a
> thread, so if we assume it is therefore exempt from those
> very-hard-to-comply-with rules, aren't we good already?  What am I
> missing?

I converted that analysis into a WIP patch, and tried to make the
Windows test setup as similar to Unix as possible.  I put in the
explanation and an assertion that it's running in another thread.
This is blind coded as I don't have Windows, but it passes CI.  I'd
probably need some help from a Windows-enabled hacker to go further
with this, though.  Does the assertion hold if you control-C the
regression test, and is there any other way to get it to fail?

The next thing is that the security infrastructure added by commit
f6dc6dd5 for CVE-2014-0067 is ripped out (because unreachable) by the
attached, but the security infrastructure added by commit be76a6d3
probably doesn't work on Windows yet.  Where src/port/mkdtemp.c does
mkdir(name, 0700), I believe Windows throws away the mode and makes a
default ACL directory, probably due to the mismatch between the
permissions models.  I haven't studied the Windows security model, but
reading tells me that AF_UNIX will obey filesystem ACLs, so I think we
should be able to make it exactly as secure as Unix if we use native
APIs.  Perhaps we just need to replace the mkdir() call in mkdtemp.c
with CreateDirectory(), passing in a locked-down owner-only
SECURITY_DESCRIPTOR, or something like that?
From 2bbaa4b37b4044f03d99b1effa137b06abfe0b21 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Fri, 19 Aug 2022 11:28:38 +1200
Subject: [PATCH 1/2] WIP: Always use Unix-domain sockets in pg_regress on
 Windows.

Since we can now rely on Unix-domain sockets working on supported
Windows releases, we can remove a difference between Unix and Windows in
pg_regress.

Previously, we thought the socket cleanup code was unsafe, so we made
Unix-domain sockets an option with a "use-at-your-own-risk" note.  On
closer inspection, the concerns about signal handlers don't seem to
apply here.  (initdb.c has similar concerns but needs separate
investigation.)

XXX This removes the code added to secure the temporary cluster by
commit f6dc6dd5, since that's based on TCP sockets.

XXX In order to secure the temporary cluster as done for Unix by commit
be76a6d3, we want to make the socket directory non-accessible to other
users.  That's done by POSIX.1-2008 mkdtemp() or our replacement code,
on Unix, which ultimately does mkdir(some_name, 0700).  The
Windows/MinGW mkdir(name, mode) macro just throws away the mode, so it
makes a default-accessible directory.  To do the equivalent thing in
Windows you'd probably need to use CreateDirectory() with an ACL that
means approximately 0700.  Research needed.
---
 src/port/mkdtemp.c            |   4 +
 src/test/regress/pg_regress.c | 274 ++++------------------------------
 2 files changed, 36 insertions(+), 242 deletions(-)

diff --git a/src/port/mkdtemp.c b/src/port/mkdtemp.c
index 8809957dcd..bf90f91848 100644
--- a/src/port/mkdtemp.c
+++ b/src/port/mkdtemp.c
@@ -187,6 +187,10 @@ GETTEMP(char *path, int *doopen, int domkdir)
 		}
 		else if (domkdir)
 		{
+			/*
+			 *  XXX Here we probably need to make a Windows ACL that allows
+			 *  only the current user, and use CreateDirectory().
+			 */
 			if (mkdir(path, 0700) >= 0)
 				return 1;
 			if (errno != EEXIST)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 9ca1a8d906..87bd030214 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -55,6 +55,20 @@ char	   *host_platform = HOST_TUPLE;
 static char *shellprog = SHELLPROG;
 #endif
 
+/*
+ * The name of the environment variable that controls where we put temporary
+ * files, to override the defaut of "/tmp".
+ */
+#ifdef WIN32
+#define TMPDIR "TMP"
+#else
+#define TMPDIR "TMPDIR"
+#endif
+
+#if defined(WIN32) && defined(USE_ASSERT_CHECKING)
+static DWORD main_thread_id;
+#endif
+
 /*
  * On Windows we use -w in diff switches to avoid problems with inconsistent
  * newline representation.  The actual result files will generally have
@@ -288,9 +302,7 @@ stop_postmaster(void)
  * postmaster exit, so it is indeterminate whether the postmaster has yet to
  * unlink the socket and lock file.  Unlink them here so we can proceed to
  * remove the directory.  Ignore errors; leaking a temporary directory is
- * unimportant.  This can run from a signal handler.  The code is not
- * acceptable in a Windows signal handler (see initdb.c:trapsig()), but
- * on Windows, pg_regress does not use Unix sockets by default.
+ * unimportant.  This can run from a signal handler.
  */
 static void
 remove_temp(void)
@@ -307,6 +319,18 @@ remove_temp(void)
 static void
 signal_remove_temp(int signum)
 {
+#ifdef WIN32
+	/*
+	 * In general, it would not be acceptable to call remove_temp() in a
+	 * Windows signal handler.  It is safe in this program though, because
+	 * SIGHUP and SIGPIPE don't really exist and SIGTERM is never raised by the
+	 * system, leaving just SIGINT.  SIGINT doesn't interrupt the main
+	 * execution context on Windows, it runs the handler concurrently in
+	 * another thread.
+	 */
+	Assert(GetCurrentThreadId() != main_thread_id);
+#endif
+
 	remove_temp();
 
 	pqsignal(signum, SIG_DFL);
@@ -329,7 +353,7 @@ static const char *
 make_temp_sockdir(void)
 {
 	char	   *template = psprintf("%s/pg_regress-XXXXXX",
-									getenv("TMPDIR") ? getenv("TMPDIR") : "/tmp");
+									getenv(TMPDIR) ? getenv(TMPDIR) : "/tmp");
 
 	temp_sockdir = mkdtemp(template);
 	if (temp_sockdir == NULL)
@@ -346,6 +370,10 @@ make_temp_sockdir(void)
 	/* Remove the directory during clean exit. */
 	atexit(remove_temp);
 
+#if defined(WIN32) && defined(USE_ASSERT_CHECKING)
+	main_thread_id = GetCurrentThreadId();
+#endif
+
 	/*
 	 * Remove the directory before dying to the usual signals.  Omit SIGQUIT,
 	 * preserving it as a quick, untidy exit.
@@ -756,211 +784,6 @@ initialize_environment(void)
 	load_resultmap();
 }
 
-#ifdef ENABLE_SSPI
-
-/* support for config_sspi_auth() */
-static const char *
-fmtHba(const char *raw)
-{
-	static char *ret;
-	const char *rp;
-	char	   *wp;
-
-	wp = ret = pg_realloc(ret, 3 + strlen(raw) * 2);
-
-	*wp++ = '"';
-	for (rp = raw; *rp; rp++)
-	{
-		if (*rp == '"')
-			*wp++ = '"';
-		*wp++ = *rp;
-	}
-	*wp++ = '"';
-	*wp++ = '\0';
-
-	return ret;
-}
-
-/*
- * Get account and domain/realm names for the current user.  This is based on
- * pg_SSPI_recvauth().  The returned strings use static storage.
- */
-static void
-current_windows_user(const char **acct, const char **dom)
-{
-	static char accountname[MAXPGPATH];
-	static char domainname[MAXPGPATH];
-	HANDLE		token;
-	TOKEN_USER *tokenuser;
-	DWORD		retlen;
-	DWORD		accountnamesize = sizeof(accountname);
-	DWORD		domainnamesize = sizeof(domainname);
-	SID_NAME_USE accountnameuse;
-
-	if (!OpenProcessToken(GetCurrentProcess(), TOKEN_READ, &token))
-	{
-		fprintf(stderr,
-				_("%s: could not open process token: error code %lu\n"),
-				progname, GetLastError());
-		exit(2);
-	}
-
-	if (!GetTokenInformation(token, TokenUser, NULL, 0, &retlen) && GetLastError() != 122)
-	{
-		fprintf(stderr,
-				_("%s: could not get token information buffer size: error code %lu\n"),
-				progname, GetLastError());
-		exit(2);
-	}
-	tokenuser = pg_malloc(retlen);
-	if (!GetTokenInformation(token, TokenUser, tokenuser, retlen, &retlen))
-	{
-		fprintf(stderr,
-				_("%s: could not get token information: error code %lu\n"),
-				progname, GetLastError());
-		exit(2);
-	}
-
-	if (!LookupAccountSid(NULL, tokenuser->User.Sid, accountname, &accountnamesize,
-						  domainname, &domainnamesize, &accountnameuse))
-	{
-		fprintf(stderr,
-				_("%s: could not look up account SID: error code %lu\n"),
-				progname, GetLastError());
-		exit(2);
-	}
-
-	free(tokenuser);
-
-	*acct = accountname;
-	*dom = domainname;
-}
-
-/*
- * Rewrite pg_hba.conf and pg_ident.conf to use SSPI authentication.  Permit
- * the current OS user to authenticate as the bootstrap superuser and as any
- * user named in a --create-role option.
- *
- * In --config-auth mode, the --user switch can be used to specify the
- * bootstrap superuser's name, otherwise we assume it is the default.
- */
-static void
-config_sspi_auth(const char *pgdata, const char *superuser_name)
-{
-	const char *accountname,
-			   *domainname;
-	char	   *errstr;
-	bool		have_ipv6;
-	char		fname[MAXPGPATH];
-	int			res;
-	FILE	   *hba,
-			   *ident;
-	_stringlist *sl;
-
-	/* Find out the name of the current OS user */
-	current_windows_user(&accountname, &domainname);
-
-	/* Determine the bootstrap superuser's name */
-	if (superuser_name == NULL)
-	{
-		/*
-		 * Compute the default superuser name the same way initdb does.
-		 *
-		 * It's possible that this result always matches "accountname", the
-		 * value SSPI authentication discovers.  But the underlying system
-		 * functions do not clearly guarantee that.
-		 */
-		superuser_name = get_user_name(&errstr);
-		if (superuser_name == NULL)
-		{
-			fprintf(stderr, "%s: %s\n", progname, errstr);
-			exit(2);
-		}
-	}
-
-	/*
-	 * Like initdb.c:setup_config(), determine whether the platform recognizes
-	 * ::1 (IPv6 loopback) as a numeric host address string.
-	 */
-	{
-		struct addrinfo *gai_result;
-		struct addrinfo hints;
-		WSADATA		wsaData;
-
-		hints.ai_flags = AI_NUMERICHOST;
-		hints.ai_family = AF_UNSPEC;
-		hints.ai_socktype = 0;
-		hints.ai_protocol = 0;
-		hints.ai_addrlen = 0;
-		hints.ai_canonname = NULL;
-		hints.ai_addr = NULL;
-		hints.ai_next = NULL;
-
-		have_ipv6 = (WSAStartup(MAKEWORD(2, 2), &wsaData) == 0 &&
-					 getaddrinfo("::1", NULL, &hints, &gai_result) == 0);
-	}
-
-	/* Check a Write outcome and report any error. */
-#define CW(cond)	\
-	do { \
-		if (!(cond)) \
-		{ \
-			fprintf(stderr, _("%s: could not write to file \"%s\": %s\n"), \
-					progname, fname, strerror(errno)); \
-			exit(2); \
-		} \
-	} while (0)
-
-	res = snprintf(fname, sizeof(fname), "%s/pg_hba.conf", pgdata);
-	if (res < 0 || res >= sizeof(fname))
-	{
-		/*
-		 * Truncating this name is a fatal error, because we must not fail to
-		 * overwrite an original trust-authentication pg_hba.conf.
-		 */
-		fprintf(stderr, _("%s: directory name too long\n"), progname);
-		exit(2);
-	}
-	hba = fopen(fname, "w");
-	if (hba == NULL)
-	{
-		fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"),
-				progname, fname, strerror(errno));
-		exit(2);
-	}
-	CW(fputs("# Configuration written by config_sspi_auth()\n", hba) >= 0);
-	CW(fputs("host all all 127.0.0.1/32  sspi include_realm=1 map=regress\n",
-			 hba) >= 0);
-	if (have_ipv6)
-		CW(fputs("host all all ::1/128  sspi include_realm=1 map=regress\n",
-				 hba) >= 0);
-	CW(fclose(hba) == 0);
-
-	snprintf(fname, sizeof(fname), "%s/pg_ident.conf", pgdata);
-	ident = fopen(fname, "w");
-	if (ident == NULL)
-	{
-		fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"),
-				progname, fname, strerror(errno));
-		exit(2);
-	}
-	CW(fputs("# Configuration written by config_sspi_auth()\n", ident) >= 0);
-
-	/*
-	 * Double-quote for the benefit of account names containing whitespace or
-	 * '#'.  Windows forbids the double-quote character itself, so don't
-	 * bother escaping embedded double-quote characters.
-	 */
-	CW(fprintf(ident, "regress  \"%s@%s\"  %s\n",
-			   accountname, domainname, fmtHba(superuser_name)) >= 0);
-	for (sl = extraroles; sl; sl = sl->next)
-		CW(fprintf(ident, "regress  \"%s@%s\"  %s\n",
-				   accountname, domainname, fmtHba(sl->str)) >= 0);
-	CW(fclose(ident) == 0);
-}
-
-#endif							/* ENABLE_SSPI */
-
 /*
  * psql_start_command, psql_add_command, psql_end_command
  *
@@ -2041,7 +1864,6 @@ regression_main(int argc, char *argv[],
 		{NULL, 0, NULL, 0}
 	};
 
-	bool		use_unix_sockets;
 	_stringlist *sl;
 	int			c;
 	int			i;
@@ -2057,20 +1879,6 @@ regression_main(int argc, char *argv[],
 
 	atexit(stop_postmaster);
 
-#if defined(WIN32)
-
-	/*
-	 * We don't use Unix-domain sockets on Windows by default (see comment at
-	 * remove_temp() for a reason).  Override at your own risk.
-	 */
-	use_unix_sockets = getenv("PG_TEST_USE_UNIX_SOCKETS") ? true : false;
-#else
-	use_unix_sockets = true;
-#endif
-
-	if (!use_unix_sockets)
-		hostname = "localhost";
-
 	/*
 	 * We call the initialization function here because that way we can set
 	 * default parameters and let them be overwritten by the commandline.
@@ -2182,13 +1990,7 @@ regression_main(int argc, char *argv[],
 	}
 
 	if (config_auth_datadir)
-	{
-#ifdef ENABLE_SSPI
-		if (!use_unix_sockets)
-			config_sspi_auth(config_auth_datadir, user);
-#endif
 		exit(0);
-	}
 
 	if (temp_instance && !port_specified_by_user)
 
@@ -2305,18 +2107,6 @@ regression_main(int argc, char *argv[],
 
 		fclose(pg_conf);
 
-#ifdef ENABLE_SSPI
-		if (!use_unix_sockets)
-		{
-			/*
-			 * Since we successfully used the same buffer for the much-longer
-			 * "initdb" command, this can't truncate.
-			 */
-			snprintf(buf, sizeof(buf), "%s/data", temp_instance);
-			config_sspi_auth(buf, NULL);
-		}
-#endif
-
 		/*
 		 * Check if there is a postmaster running already.
 		 */
-- 
2.35.1

From 9df705e98ef6197f3c9abd24a66ca2b299ba6d74 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Fri, 19 Aug 2022 12:00:07 +1200
Subject: [PATCH 2/2] WIP: Stop using TCP in TAP tests on Windows.

Since Unix-domain sockets are available on our minimum target Windows
version, we can remove a source of instability and a point of variation
between Unix and Windows.
---
 .cirrus.yml                               |  3 -
 src/bin/pg_ctl/t/001_start_stop.pl        | 13 +---
 src/test/authentication/t/001_password.pl |  6 --
 src/test/authentication/t/002_saslprep.pl |  7 --
 src/test/perl/PostgreSQL/Test/Cluster.pm  | 91 ++++-------------------
 src/test/perl/PostgreSQL/Test/Utils.pm    |  9 +--
 6 files changed, 20 insertions(+), 109 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 81eb8a9996..8890f660c9 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -361,9 +361,6 @@ task:
     # git's tar doesn't deal with drive letters, see
     # https://postgr.es/m/b6782dc3-a7b0-ed56-175f-f8f54cb08d67%40dunslane.net
     TAR: "c:/windows/system32/tar.exe"
-    # Avoids port conflicts between concurrent tap test runs
-    PG_TEST_USE_UNIX_SOCKETS: 1
-    PG_REGRESS_SOCK_DIR: "c:/cirrus/"
     # -m enables parallelism
     # verbosity:minimal + Summary reduce verbosity, while keeping a summary of
     #   errors/warnings
diff --git a/src/bin/pg_ctl/t/001_start_stop.pl b/src/bin/pg_ctl/t/001_start_stop.pl
index fdffd76d99..23f942a440 100644
--- a/src/bin/pg_ctl/t/001_start_stop.pl
+++ b/src/bin/pg_ctl/t/001_start_stop.pl
@@ -29,16 +29,9 @@ print $conf "port = $node_port\n";
 print $conf PostgreSQL::Test::Utils::slurp_file($ENV{TEMP_CONFIG})
   if defined $ENV{TEMP_CONFIG};
 
-if ($use_unix_sockets)
-{
-	print $conf "listen_addresses = ''\n";
-	$tempdir_short =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
-	print $conf "unix_socket_directories = '$tempdir_short'\n";
-}
-else
-{
-	print $conf "listen_addresses = '127.0.0.1'\n";
-}
+print $conf "listen_addresses = ''\n";
+$tempdir_short =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
+print $conf "unix_socket_directories = '$tempdir_short'\n";
 close $conf;
 my $ctlcmd = [
 	'pg_ctl', 'start', '-D', "$tempdir/data", '-l',
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index 3e3079c824..3e1c1a070b 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -6,18 +6,12 @@
 # - Plain
 # - MD5-encrypted
 # - SCRAM-encrypted
-# This test can only run with Unix-domain sockets.
 
 use strict;
 use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 use Test::More;
-if (!$use_unix_sockets)
-{
-	plan skip_all =>
-	  "authentication tests cannot run without Unix-domain sockets";
-}
 
 # Delete pg_hba.conf from the given node, add a new entry to it
 # and then execute a reload to refresh it.
diff --git a/src/test/authentication/t/002_saslprep.pl b/src/test/authentication/t/002_saslprep.pl
index 5e87e21ee9..23849632c2 100644
--- a/src/test/authentication/t/002_saslprep.pl
+++ b/src/test/authentication/t/002_saslprep.pl
@@ -2,19 +2,12 @@
 # Copyright (c) 2021-2022, PostgreSQL Global Development Group
 
 # Test password normalization in SCRAM.
-#
-# This test can only run with Unix-domain sockets.
 
 use strict;
 use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 use Test::More;
-if (!$use_unix_sockets)
-{
-	plan skip_all =>
-	  "authentication tests cannot run without Unix-domain sockets";
-}
 
 # Delete pg_hba.conf from the given node, add a new entry to it
 # and then execute a reload to refresh it.
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 27fa607da4..6fdf73db2d 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -108,7 +108,7 @@ use PostgreSQL::Test::Utils ();
 use Time::HiRes qw(usleep);
 use Scalar::Util qw(blessed);
 
-our ($use_tcp, $test_localhost, $test_pghost, $last_host_assigned,
+our ($test_pghost,
 	$last_port_assigned, @all_nodes, $died);
 
 # the minimum version we believe to be compatible with this package without
@@ -120,21 +120,11 @@ INIT
 
 	# Set PGHOST for backward compatibility.  This doesn't work for own_host
 	# nodes, so prefer to not rely on this when writing new tests.
-	$use_tcp            = !$PostgreSQL::Test::Utils::use_unix_sockets;
-	$test_localhost     = "127.0.0.1";
-	$last_host_assigned = 1;
-	if ($use_tcp)
-	{
-		$test_pghost = $test_localhost;
-	}
-	else
-	{
-		# On windows, replace windows-style \ path separators with / when
-		# putting socket directories either in postgresql.conf or libpq
-		# connection strings, otherwise they are interpreted as escapes.
-		$test_pghost = PostgreSQL::Test::Utils::tempdir_short;
-		$test_pghost =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
-	}
+	# On windows, replace windows-style \ path separators with / when
+	# putting socket directories either in postgresql.conf or libpq
+	# connection strings, otherwise they are interpreted as escapes.
+	$test_pghost = PostgreSQL::Test::Utils::tempdir_short;
+	$test_pghost =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
 	$ENV{PGHOST}     = $test_pghost;
 	$ENV{PGDATABASE} = 'postgres';
 
@@ -425,12 +415,6 @@ sub set_replication_conf
 	open my $hba, '>>', "$pgdata/pg_hba.conf";
 	print $hba
 	  "\n# Allow replication (set up by PostgreSQL::Test::Cluster.pm)\n";
-	if ($PostgreSQL::Test::Utils::windows_os
-		&& !$PostgreSQL::Test::Utils::use_unix_sockets)
-	{
-		print $hba
-		  "host replication all $test_localhost/32 sspi include_realm=1 map=regress\n";
-	}
 	close $hba;
 	return;
 }
@@ -523,16 +507,8 @@ sub init
 	}
 
 	print $conf "port = $port\n";
-	if ($use_tcp)
-	{
-		print $conf "unix_socket_directories = ''\n";
-		print $conf "listen_addresses = '$host'\n";
-	}
-	else
-	{
-		print $conf "unix_socket_directories = '$host'\n";
-		print $conf "listen_addresses = ''\n";
-	}
+	print $conf "unix_socket_directories = '$host'\n";
+	print $conf "listen_addresses = ''\n";
 	close $conf;
 
 	chmod($self->group_access ? 0640 : 0600, "$pgdata/postgresql.conf")
@@ -751,15 +727,8 @@ sub init_from_backup
 		qq(
 port = $port
 ));
-	if ($use_tcp)
-	{
-		$self->append_conf('postgresql.conf', "listen_addresses = '$host'");
-	}
-	else
-	{
-		$self->append_conf('postgresql.conf',
-			"unix_socket_directories = '$host'");
-	}
+	$self->append_conf('postgresql.conf',
+		"unix_socket_directories = '$host'");
 	$self->enable_streaming($root_node) if $params{has_streaming};
 	$self->enable_restoring($root_node, $params{standby})
 	  if $params{has_restoring};
@@ -1225,9 +1194,7 @@ sub new
 	else
 	{
 		# When selecting a port, we look for an unassigned TCP port number,
-		# even if we intend to use only Unix-domain sockets.  This is clearly
-		# necessary on $use_tcp (Windows) configurations, and it seems like a
-		# good idea on Unixen as well.
+		# even if we intend to use only Unix-domain sockets.
 		$port = get_free_port();
 	}
 
@@ -1235,17 +1202,8 @@ sub new
 	my $host = $test_pghost;
 	if ($params{own_host})
 	{
-		if ($use_tcp)
-		{
-			$last_host_assigned++;
-			$last_host_assigned > 254 and BAIL_OUT("too many own_host nodes");
-			$host = '127.0.0.' . $last_host_assigned;
-		}
-		else
-		{
-			$host = "$test_pghost/$name"; # Assume $name =~ /^[-_a-zA-Z0-9]+$/
-			mkdir $host;
-		}
+		$host = "$test_pghost/$name"; # Assume $name =~ /^[-_a-zA-Z0-9]+$/
+		mkdir $host;
 	}
 
 	my $testname = basename($0);
@@ -1452,6 +1410,7 @@ start other, non-Postgres servers.
 Ports assigned to existing PostgreSQL::Test::Cluster objects are automatically
 excluded, even if those servers are not currently running.
 
+
 XXX A port available now may become unavailable by the time we start
 the desired service.
 
@@ -1481,29 +1440,11 @@ sub get_free_port
 		}
 
 		# Check to see if anything else is listening on this TCP port.
-		# Seek a port available for all possible listen_addresses values,
-		# so callers can harness this port for the widest range of purposes.
-		# The 0.0.0.0 test achieves that for MSYS, which automatically sets
-		# SO_EXCLUSIVEADDRUSE.  Testing 0.0.0.0 is insufficient for Windows
-		# native Perl (https://stackoverflow.com/a/14388707), so we also
-		# have to test individual addresses.  Doing that for 127.0.0/24
-		# addresses other than 127.0.0.1 might fail with EADDRNOTAVAIL on
-		# non-Linux, non-Windows kernels.
-		#
-		# Thus, 0.0.0.0 and individual 127.0.0/24 addresses are tested
-		# only on Windows and only when TCP usage is requested.
 		if ($found == 1)
 		{
-			foreach my $addr (qw(127.0.0.1),
-				($use_tcp && $PostgreSQL::Test::Utils::windows_os)
-				  ? qw(127.0.0.2 127.0.0.3 0.0.0.0)
-				  : ())
+			if (!can_bind(qw(127.0.0.1), $port))
 			{
-				if (!can_bind($addr, $port))
-				{
-					$found = 0;
-					last;
-				}
+				$found = 0;
 			}
 		}
 	}
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm
index 1ca2cc5917..40968176fc 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -88,10 +88,9 @@ our @EXPORT = qw(
 
   $windows_os
   $is_msys2
-  $use_unix_sockets
 );
 
-our ($windows_os, $is_msys2, $use_unix_sockets, $timeout_default,
+our ($windows_os, $is_msys2, $timeout_default,
 	$tmp_check, $log_path, $test_logfile);
 
 BEGIN
@@ -153,12 +152,6 @@ BEGIN
 		Win32API::File->import(qw(createFile OsFHandleOpen CloseHandle));
 	}
 
-	# Specifies whether to use Unix sockets for test setups.  On
-	# Windows we don't use them by default since it's not universally
-	# supported, but it can be overridden if desired.
-	$use_unix_sockets =
-	  (!$windows_os || defined $ENV{PG_TEST_USE_UNIX_SOCKETS});
-
 	$timeout_default = $ENV{PG_TEST_TIMEOUT_DEFAULT};
 	$timeout_default = 180
 	  if not defined $timeout_default or $timeout_default eq '';
-- 
2.35.1

Reply via email to