Revert "Consistently test for in-use shared memory."

This reverts commits 2f932f71,
16ee6eaf and
6f0e1900.  The buildfarm has revealed
several bugs.  Back-patch like the original commits.

Discussion: https://postgr.es/m/20190404145319.GA1720877@rfd.leadboat.com

src/Makefile.global.in

@@ -430,13 +430,13 @@ ifeq ($(enable_tap_tests),yes)
 define prove_installcheck
 rm -rf '$(CURDIR)'/tmp_check
 $(MKDIR_P) '$(CURDIR)'/tmp_check
-cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(bindir):$$PATH" PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' REGRESS_SHLIB='$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
+cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(bindir):$$PATH" PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
 endef
 
 define prove_check
 rm -rf '$(CURDIR)'/tmp_check
 $(MKDIR_P) '$(CURDIR)'/tmp_check
-cd $(srcdir) && TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' REGRESS_SHLIB='$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
+cd $(srcdir) && TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
 endef
 
 else

src/backend/port/win32_shmem.c

@@ -170,9 +170,14 @@ EnableLockPagesPrivilege(int elevel)
  *
  * Create a shared memory segment of the given size and initialize its
  * standard header.
+ *
+ * makePrivate means to always create a new segment, rather than attach to
+ * or recycle any existing segment. On win32, we always create a new segment,
+ * since there is no need for recycling (segments go away automatically
+ * when the last backend exits)
  */
 PGShmemHeader *
-PGSharedMemoryCreate(Size size, int port,
+PGSharedMemoryCreate(Size size, bool makePrivate, int port,
 					 PGShmemHeader **shim)
 {
 	void	   *memAddress;

src/backend/postmaster/postmaster.c

@@ -2604,7 +2604,7 @@ reset_shared(int port)
 	 * determine IPC keys.  This helps ensure that we will clean up dead IPC
 	 * objects if the postmaster crashes and is restarted.
 	 */
-	CreateSharedMemoryAndSemaphores(port);
+	CreateSharedMemoryAndSemaphores(false, port);
 }
 
 
@@ -4945,7 +4945,7 @@ SubPostmasterMain(int argc, char *argv[])
 		InitProcess();
 
 		/* Attach process to shared data structures */
-		CreateSharedMemoryAndSemaphores(0);
+		CreateSharedMemoryAndSemaphores(false, 0);
 
 		/* And run the backend */
 		BackendRun(&port);		/* does not return */
@@ -4959,7 +4959,7 @@ SubPostmasterMain(int argc, char *argv[])
 		InitAuxiliaryProcess();
 
 		/* Attach process to shared data structures */
-		CreateSharedMemoryAndSemaphores(0);
+		CreateSharedMemoryAndSemaphores(false, 0);
 
 		AuxiliaryProcessMain(argc - 2, argv + 2);	/* does not return */
 	}
@@ -4972,7 +4972,7 @@ SubPostmasterMain(int argc, char *argv[])
 		InitProcess();
 
 		/* Attach process to shared data structures */
-		CreateSharedMemoryAndSemaphores(0);
+		CreateSharedMemoryAndSemaphores(false, 0);
 
 		AutoVacLauncherMain(argc - 2, argv + 2);	/* does not return */
 	}
@@ -4985,7 +4985,7 @@ SubPostmasterMain(int argc, char *argv[])
 		InitProcess();
 
 		/* Attach process to shared data structures */
-		CreateSharedMemoryAndSemaphores(0);
+		CreateSharedMemoryAndSemaphores(false, 0);
 
 		AutoVacWorkerMain(argc - 2, argv + 2);	/* does not return */
 	}
@@ -5003,7 +5003,7 @@ SubPostmasterMain(int argc, char *argv[])
 		InitProcess();
 
 		/* Attach process to shared data structures */
-		CreateSharedMemoryAndSemaphores(0);
+		CreateSharedMemoryAndSemaphores(false, 0);
 
 		/* Fetch MyBgworkerEntry from shared memory */
 		shmem_slot = atoi(argv[1] + 15);

src/backend/storage/ipc/ipci.c

@@ -89,9 +89,12 @@ RequestAddinShmemSpace(Size size)
  * through the same code as before.  (Note that the called routines mostly
  * check IsUnderPostmaster, rather than EXEC_BACKEND, to detect this case.
  * This is a bit code-wasteful and could be cleaned up.)
+ *
+ * If "makePrivate" is true then we only need private memory, not shared
+ * memory.  This is true for a standalone backend, false for a postmaster.
  */
 void
-CreateSharedMemoryAndSemaphores(int port)
+CreateSharedMemoryAndSemaphores(bool makePrivate, int port)
 {
 	PGShmemHeader *shim = NULL;
 
@@ -163,7 +166,7 @@ CreateSharedMemoryAndSemaphores(int port)
 		/*
 		 * Create the shmem segment
 		 */
-		seghdr = PGSharedMemoryCreate(size, port, &shim);
+		seghdr = PGSharedMemoryCreate(size, makePrivate, port, &shim);
 
 		InitShmemAccess(seghdr);
 
@@ -184,9 +187,12 @@ CreateSharedMemoryAndSemaphores(int port)
 	{
 		/*
 		 * We are reattaching to an existing shared memory segment. This
-		 * should only be reached in the EXEC_BACKEND case.
+		 * should only be reached in the EXEC_BACKEND case, and even then only
+		 * with makePrivate == false.
 		 */
-#ifndef EXEC_BACKEND
+#ifdef EXEC_BACKEND
+		Assert(!makePrivate);
+#else
 		elog(PANIC, "should be attached to shared memory already");
 #endif
 	}

src/backend/utils/init/postinit.c

@@ -446,11 +446,9 @@ InitCommunication(void)
 	{
 		/*
 		 * We're running a postgres bootstrap process or a standalone backend.
-		 * Though we won't listen on PostPortNumber, use it to select a shmem
-		 * key.  This increases the chance of detecting a leftover live
-		 * backend of this DataDir.
+		 * Create private "shmem" and semaphores.
 		 */
-		CreateSharedMemoryAndSemaphores(PostPortNumber);
+		CreateSharedMemoryAndSemaphores(true, 0);
 	}
 }
 

src/include/storage/ipc.h

@@ -76,6 +76,6 @@ extern void on_exit_reset(void);
 /* ipci.c */
 extern PGDLLIMPORT shmem_startup_hook_type shmem_startup_hook;
 
-extern void CreateSharedMemoryAndSemaphores(int port);
+extern void CreateSharedMemoryAndSemaphores(bool makePrivate, int port);
 
 #endif							/* IPC_H */

src/include/storage/pg_shmem.h

@@ -30,7 +30,7 @@ typedef struct PGShmemHeader	/* standard header for all Postgres shmem */
 {
 	int32		magic;			/* magic # to identify Postgres segments */
 #define PGShmemMagic  679834894
-	pid_t		creatorPID;		/* PID of creating process (set but unread) */
+	pid_t		creatorPID;		/* PID of creating process */
 	Size		totalsize;		/* total size of segment */
 	Size		freeoffset;		/* offset to first free space */
 	dsm_handle	dsm_control;	/* ID of dynamic shared memory control seg */
@@ -81,8 +81,8 @@ extern void PGSharedMemoryReAttach(void);
 extern void PGSharedMemoryNoReAttach(void);
 #endif
 
-extern PGShmemHeader *PGSharedMemoryCreate(Size size, int port,
-					 PGShmemHeader **shim);
+extern PGShmemHeader *PGSharedMemoryCreate(Size size, bool makePrivate,
+					 int port, PGShmemHeader **shim);
 extern bool PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2);
 extern void PGSharedMemoryDetach(void);
 

src/test/recovery/t/017_shm.pl

-#
-# Tests of pg_shmem.h functions
-#
-use strict;
-use warnings;
-use IPC::Run 'run';
-use PostgresNode;
-use Test::More;
-use TestLib;
-
-plan tests => 6;
-
-my $tempdir = TestLib::tempdir;
-my $port;
-
-# Log "ipcs" diffs on a best-effort basis, swallowing any error.
-my $ipcs_before = "$tempdir/ipcs_before";
-eval { run_log [ 'ipcs', '-am' ], '>', $ipcs_before; };
-
-sub log_ipcs
-{
-	eval { run_log [ 'ipcs', '-am' ], '|', [ 'diff', $ipcs_before, '-' ] };
-	return;
-}
-
-# With Unix sockets, choose a port number such that the port number's first
-# IpcMemoryKey candidate is available.  If multiple copies of this test run
-# concurrently, they will pick different ports.  In the absence of collisions
-# from other shmget() activity, gnat starts with key 0x7d001 (512001), and
-# flea starts with key 0x7d002 (512002).  With TCP, the first get_new_node
-# picks a port number.
-my $port_holder;
-if (!$PostgresNode::use_tcp)
-{
-	for ($port = 512; $port < 612; ++$port)
-	{
-		$port_holder = PostgresNode->get_new_node(
-			"port${port}_holder",
-			port     => $port,
-			own_host => 1);
-		$port_holder->init;
-		$port_holder->start;
-		# Match the AddToDataDirLockFile() call in sysv_shmem.c.  Assume all
-		# systems not using sysv_shmem.c do use TCP.
-		my $shmem_key_line_prefix = sprintf("%9lu ", 1 + $port * 1000);
-		last
-		  if slurp_file($port_holder->data_dir . '/postmaster.pid') =~
-		  /^$shmem_key_line_prefix/m;
-		$port_holder->stop;
-	}
-}
-
-# Node setup.
-sub init_start
-{
-	my $name = shift;
-	my $ret = PostgresNode->get_new_node($name, port => $port, own_host => 1);
-	defined($port) or $port = $ret->port;    # same port for all nodes
-	$ret->init;
-	$ret->start;
-	log_ipcs();
-	return $ret;
-}
-my $gnat = init_start 'gnat';
-my $flea = init_start 'flea';
-
-# Upon postmaster death, postmaster children exit automatically.
-$gnat->kill9;
-log_ipcs();
-$flea->restart;       # flea ignores the shm key gnat abandoned.
-log_ipcs();
-poll_start($gnat);    # gnat recycles its former shm key.
-log_ipcs();
-
-# After clean shutdown, the nodes swap shm keys.
-$gnat->stop;
-$flea->restart;
-log_ipcs();
-$gnat->start;
-log_ipcs();
-
-# Scenarios involving no postmaster.pid, dead postmaster, and a live backend.
-# Use a regress.c function to emulate the responsiveness of a backend working
-# through a CPU-intensive task.
-$gnat->safe_psql('postgres', <<EOSQL);
-CREATE FUNCTION wait_pid(int)
-   RETURNS void
-   AS '$ENV{REGRESS_SHLIB}'
-   LANGUAGE C STRICT;
-EOSQL
-my $slow_query = 'SELECT wait_pid(pg_backend_pid())';
-my ($stdout, $stderr);
-my $slow_client = IPC::Run::start(
-	[
-		'psql', '-X', '-qAt', '-d', $gnat->connstr('postgres'),
-		'-c', $slow_query
-	],
-	'<',
-	\undef,
-	'>',
-	\$stdout,
-	'2>',
-	\$stderr,
-	IPC::Run::timeout(900));    # five times the poll_query_until timeout
-ok( $gnat->poll_query_until(
-		'postgres',
-		"SELECT 1 FROM pg_stat_activity WHERE query = '$slow_query'", '1'),
-	'slow query started');
-my $slow_pid = $gnat->safe_psql('postgres',
-	"SELECT pid FROM pg_stat_activity WHERE query = '$slow_query'");
-$gnat->kill9;
-unlink($gnat->data_dir . '/postmaster.pid');
-$gnat->rotate_logfile;    # on Windows, can't open old log for writing
-log_ipcs();
-# Reject ordinary startup.
-ok(!$gnat->start(fail_ok => 1), 'live query blocks restart');
-like(
-	slurp_file($gnat->logfile),
-	qr/pre-existing shared memory block/,
-	'detected live backend via shared memory');
-# Reject single-user startup.
-my $single_stderr;
-ok( !run_log(
-		[ 'postgres', '--single', '-D', $gnat->data_dir, 'template1' ],
-		'<', \('SELECT 1 + 1'), '2>', \$single_stderr),
-	'live query blocks --single');
-print STDERR $single_stderr;
-like(
-	$single_stderr,
-	qr/pre-existing shared memory block/,
-	'single-user mode detected live backend via shared memory');
-log_ipcs();
-# Fail to reject startup if shm key N has become available and we crash while
-# using key N+1.  This is unwanted, but expected.  Windows is immune, because
-# its GetSharedMemName() use DataDir strings, not numeric keys.
-$flea->stop;    # release first key
-is( $gnat->start(fail_ok => 1),
-	$TestLib::windows_os ? 0 : 1,
-	'key turnover fools only sysv_shmem.c');
-$gnat->stop;     # release first key (no-op on $TestLib::windows_os)
-$flea->start;    # grab first key
-# cleanup
-TestLib::system_log('pg_ctl', 'kill', 'QUIT', $slow_pid);
-$slow_client->finish;    # client has detected backend termination
-log_ipcs();
-poll_start($gnat);       # recycle second key
-
-$gnat->stop;
-$flea->stop;
-$port_holder->stop if $port_holder;
-log_ipcs();
-
-
-# When postmaster children are slow to exit after postmaster death, we may
-# need retries to start a new postmaster.
-sub poll_start
-{
-	my ($node) = @_;
-
-	my $max_attempts = 180 * 10;
-	my $attempts     = 0;
-
-	while ($attempts < $max_attempts)
-	{
-		$node->start(fail_ok => 1) && return 1;
-
-		# Wait 0.1 second before retrying.
-		usleep(100_000);
-
-		$attempts++;
-	}
-
-	# No success within 180 seconds.  Try one last time without fail_ok, which
-	# will BAIL_OUT unless it succeeds.
-	$node->start && return 1;
-	return 0;
-}

src/tools/msvc/vcregress.pl

@@ -205,7 +205,6 @@ sub tap_check
 	local %ENV = %ENV;
 	$ENV{PERL5LIB}   = "$topdir/src/test/perl;$ENV{PERL5LIB}";
 	$ENV{PG_REGRESS} = "$topdir/$Config/pg_regress/pg_regress";
-	$ENV{REGRESS_SHLIB} = "$topdir/src/test/regress/regress.dll";
 
 	$ENV{TESTDIR} = "$dir";