diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 64ce7557ba..ad3559e0a7 100644 --- a/src/Makefile.global.in +++ b/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 diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c index a4533046ce..741c455ccb 100644 --- a/src/backend/port/sysv_shmem.c +++ b/src/backend/port/sysv_shmem.c @@ -71,26 +71,6 @@ typedef key_t IpcMemoryKey; /* shared memory key passed to shmget(2) */ typedef int IpcMemoryId; /* shared memory ID returned by shmget(2) */ -/* - * How does a given IpcMemoryId relate to this PostgreSQL process? - * - * One could recycle unattached segments of different data directories if we - * distinguished that case from other SHMSTATE_FOREIGN cases. Doing so would - * cause us to visit less of the key space, making us less likely to detect a - * SHMSTATE_ATTACHED key. It would also complicate the concurrency analysis, - * in that postmasters of different data directories could simultaneously - * attempt to recycle a given key. We'll waste keys longer in some cases, but - * avoiding the problems of the alternative justifies that loss. - */ -typedef enum -{ - SHMSTATE_ANALYSIS_FAILURE, /* unexpected failure to analyze the ID */ - SHMSTATE_ATTACHED, /* pertinent to DataDir, has attached PIDs */ - SHMSTATE_ENOENT, /* no segment of that ID */ - SHMSTATE_FOREIGN, /* exists, but not pertinent to DataDir */ - SHMSTATE_UNATTACHED /* pertinent to DataDir, no attached PIDs */ -} IpcMemoryState; - unsigned long UsedShmemSegID = 0; void *UsedShmemSegAddr = NULL; @@ -103,8 +83,8 @@ static void *AnonymousShmem = NULL; static void *InternalIpcMemoryCreate(IpcMemoryKey memKey, Size size); static void IpcMemoryDetach(int status, Datum shmaddr); static void IpcMemoryDelete(int status, Datum shmId); -static IpcMemoryState PGSharedMemoryAttach(IpcMemoryId shmId, - PGShmemHeader **addr); +static PGShmemHeader *PGSharedMemoryAttach(IpcMemoryKey key, + IpcMemoryId *shmid); /* @@ -308,36 +288,11 @@ IpcMemoryDelete(int status, Datum shmId) bool PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2) { - PGShmemHeader *memAddress; - IpcMemoryState state; - - state = PGSharedMemoryAttach((IpcMemoryId) id2, &memAddress); - if (memAddress && shmdt(memAddress) < 0) - elog(LOG, "shmdt(%p) failed: %m", memAddress); - switch (state) - { - case SHMSTATE_ENOENT: - case SHMSTATE_FOREIGN: - case SHMSTATE_UNATTACHED: - return false; - case SHMSTATE_ANALYSIS_FAILURE: - case SHMSTATE_ATTACHED: - return true; - } - return true; -} - -/* See comment at IpcMemoryState. */ -static IpcMemoryState -PGSharedMemoryAttach(IpcMemoryId shmId, - PGShmemHeader **addr) -{ + IpcMemoryId shmId = (IpcMemoryId) id2; struct shmid_ds shmStat; struct stat statbuf; PGShmemHeader *hdr; - *addr = NULL; - /* * We detect whether a shared memory segment is in use by seeing whether * it (a) exists and (b) has any processes attached to it. @@ -350,7 +305,7 @@ PGSharedMemoryAttach(IpcMemoryId shmId, * exists. */ if (errno == EINVAL) - return SHMSTATE_ENOENT; + return false; /* * EACCES implies that the segment belongs to some other userid, which @@ -358,7 +313,7 @@ PGSharedMemoryAttach(IpcMemoryId shmId, * is relevant to our data directory). */ if (errno == EACCES) - return SHMSTATE_FOREIGN; + return false; /* * Some Linux kernel versions (in fact, all of them as of July 2007) @@ -369,7 +324,7 @@ PGSharedMemoryAttach(IpcMemoryId shmId, */ #ifdef HAVE_LINUX_EIDRM_BUG if (errno == EIDRM) - return SHMSTATE_ENOENT; + return false; #endif /* @@ -377,26 +332,25 @@ PGSharedMemoryAttach(IpcMemoryId shmId, * only likely case is EIDRM, which implies that the segment has been * IPC_RMID'd but there are still processes attached to it. */ - return SHMSTATE_ANALYSIS_FAILURE; + return true; } + /* If it has no attached processes, it's not in use */ + if (shmStat.shm_nattch == 0) + return false; + /* * Try to attach to the segment and see if it matches our data directory. * This avoids shmid-conflict problems on machines that are running * several postmasters under the same userid. */ if (stat(DataDir, &statbuf) < 0) - return SHMSTATE_ANALYSIS_FAILURE; /* can't stat; be conservative */ + return true; /* if can't stat, be conservative */ + + hdr = (PGShmemHeader *) shmat(shmId, NULL, PG_SHMAT_FLAGS); - /* - * If we can't attach, be conservative. This may fail if postmaster.pid - * furnished the shmId and another user created a world-readable segment - * of the same shmId. - */ - hdr = (PGShmemHeader *) shmat(shmId, UsedShmemSegAddr, PG_SHMAT_FLAGS); if (hdr == (PGShmemHeader *) -1) - return SHMSTATE_ANALYSIS_FAILURE; - *addr = hdr; + return true; /* if can't attach, be conservative */ if (hdr->magic != PGShmemMagic || hdr->device != statbuf.st_dev || @@ -404,12 +358,16 @@ PGSharedMemoryAttach(IpcMemoryId shmId, { /* * It's either not a Postgres segment, or not one for my data - * directory. + * directory. In either case it poses no threat. */ - return SHMSTATE_FOREIGN; + shmdt((void *) hdr); + return false; } - return shmStat.shm_nattch == 0 ? SHMSTATE_UNATTACHED : SHMSTATE_ATTACHED; + /* Trouble --- looks a lot like there's still live backends */ + shmdt((void *) hdr); + + return true; } #ifdef USE_ANONYMOUS_SHMEM @@ -585,21 +543,25 @@ AnonymousShmemDetach(int status, Datum arg) * standard header. Also, register an on_shmem_exit callback to release * the storage. * - * Dead Postgres segments pertinent to this DataDir are recycled if found, but - * we do not fail upon collision with foreign shmem segments. The idea here - * is to detect and re-use keys that may have been assigned by a crashed - * postmaster or backend. + * Dead Postgres segments are recycled if found, but we do not fail upon + * collision with non-Postgres shmem segments. The idea here is to detect and + * re-use keys that may have been assigned by a crashed postmaster or backend. + * + * makePrivate means to always create a new segment, rather than attach to + * or recycle any existing segment. * * The port number is passed for possible use as a key (for SysV, we use - * it to generate the starting shmem key). + * it to generate the starting shmem key). In a standalone backend, + * zero will be passed. */ PGShmemHeader * -PGSharedMemoryCreate(Size size, int port, +PGSharedMemoryCreate(Size size, bool makePrivate, int port, PGShmemHeader **shim) { IpcMemoryKey NextShmemSegID; void *memAddress; PGShmemHeader *hdr; + IpcMemoryId shmid; struct stat statbuf; Size sysvsize; @@ -630,20 +592,11 @@ PGSharedMemoryCreate(Size size, int port, /* Make sure PGSharedMemoryAttach doesn't fail without need */ UsedShmemSegAddr = NULL; - /* - * Loop till we find a free IPC key. Trust CreateDataDirLockFile() to - * ensure no more than one postmaster per data directory can enter this - * loop simultaneously. (CreateDataDirLockFile() does not ensure that, - * but prefer fixing it over coping here.) - */ - NextShmemSegID = 1 + port * 1000; + /* Loop till we find a free IPC key */ + NextShmemSegID = port * 1000; - for (;;) + for (NextShmemSegID++;; NextShmemSegID++) { - IpcMemoryId shmid; - PGShmemHeader *oldhdr; - IpcMemoryState state; - /* Try to create new segment */ memAddress = InternalIpcMemoryCreate(NextShmemSegID, sysvsize); if (memAddress) @@ -651,71 +604,58 @@ PGSharedMemoryCreate(Size size, int port, /* Check shared memory and possibly remove and recreate */ + if (makePrivate) /* a standalone backend shouldn't do this */ + continue; + + if ((memAddress = PGSharedMemoryAttach(NextShmemSegID, &shmid)) == NULL) + continue; /* can't attach, not one of mine */ + /* - * shmget() failure is typically EACCES, hence SHMSTATE_FOREIGN. - * ENOENT, a narrow possibility, implies SHMSTATE_ENOENT, but one can - * safely treat SHMSTATE_ENOENT like SHMSTATE_FOREIGN. + * If I am not the creator and it belongs to an extant process, + * continue. */ - shmid = shmget(NextShmemSegID, sizeof(PGShmemHeader), 0); - if (shmid < 0) + hdr = (PGShmemHeader *) memAddress; + if (hdr->creatorPID != getpid()) { - oldhdr = NULL; - state = SHMSTATE_FOREIGN; - } - else - state = PGSharedMemoryAttach(shmid, &oldhdr); - - switch (state) - { - case SHMSTATE_ANALYSIS_FAILURE: - case SHMSTATE_ATTACHED: - ereport(FATAL, - (errcode(ERRCODE_LOCK_FILE_EXISTS), - errmsg("pre-existing shared memory block (key %lu, ID %lu) is still in use", - (unsigned long) NextShmemSegID, - (unsigned long) shmid), - errhint("Terminate any old server processes associated with data directory \"%s\".", - DataDir))); - break; - case SHMSTATE_ENOENT: - - /* - * To our surprise, some other process deleted since our last - * InternalIpcMemoryCreate(). Moments earlier, we would have - * seen SHMSTATE_FOREIGN. Try that same ID again. - */ - elog(LOG, - "shared memory block (key %lu, ID %lu) deleted during startup", - (unsigned long) NextShmemSegID, - (unsigned long) shmid); - break; - case SHMSTATE_FOREIGN: - NextShmemSegID++; - break; - case SHMSTATE_UNATTACHED: - - /* - * The segment pertains to DataDir, and every process that had - * used it has died or detached. Zap it, if possible, and any - * associated dynamic shared memory segments, as well. This - * shouldn't fail, but if it does, assume the segment belongs - * to someone else after all, and try the next candidate. - * Otherwise, try again to create the segment. That may fail - * if some other process creates the same shmem key before we - * do, in which case we'll try the next key. - */ - if (oldhdr->dsm_control != 0) - dsm_cleanup_using_control_segment(oldhdr->dsm_control); - if (shmctl(shmid, IPC_RMID, NULL) < 0) - NextShmemSegID++; - break; + if (kill(hdr->creatorPID, 0) == 0 || errno != ESRCH) + { + shmdt(memAddress); + continue; /* segment belongs to a live process */ + } } - if (oldhdr && shmdt(oldhdr) < 0) - elog(LOG, "shmdt(%p) failed: %m", oldhdr); + /* + * The segment appears to be from a dead Postgres process, or from a + * previous cycle of life in this same process. Zap it, if possible, + * and any associated dynamic shared memory segments, as well. This + * probably shouldn't fail, but if it does, assume the segment belongs + * to someone else after all, and continue quietly. + */ + if (hdr->dsm_control != 0) + dsm_cleanup_using_control_segment(hdr->dsm_control); + shmdt(memAddress); + if (shmctl(shmid, IPC_RMID, NULL) < 0) + continue; + + /* + * Now try again to create the segment. + */ + memAddress = InternalIpcMemoryCreate(NextShmemSegID, sysvsize); + if (memAddress) + break; /* successful create and attach */ + + /* + * Can only get here if some other process managed to create the same + * shmem key before we did. Let him have that one, loop around to try + * next key. + */ } - /* Initialize new segment. */ + /* + * OK, we created a new segment. Mark it as created by this process. The + * order of assignments here is critical so that another Postgres process + * can't see the header as valid but belonging to an invalid PID! + */ hdr = (PGShmemHeader *) memAddress; hdr->creatorPID = getpid(); hdr->magic = PGShmemMagic; @@ -775,8 +715,7 @@ void PGSharedMemoryReAttach(void) { IpcMemoryId shmid; - PGShmemHeader *hdr; - IpcMemoryState state; + void *hdr; void *origUsedShmemSegAddr = UsedShmemSegAddr; Assert(UsedShmemSegAddr != NULL); @@ -789,18 +728,14 @@ PGSharedMemoryReAttach(void) #endif elog(DEBUG3, "attaching to %p", UsedShmemSegAddr); - shmid = shmget(UsedShmemSegID, sizeof(PGShmemHeader), 0); - if (shmid < 0) - state = SHMSTATE_FOREIGN; - else - state = PGSharedMemoryAttach(shmid, &hdr); - if (state != SHMSTATE_ATTACHED) + hdr = (void *) PGSharedMemoryAttach((IpcMemoryKey) UsedShmemSegID, &shmid); + if (hdr == NULL) elog(FATAL, "could not reattach to shared memory (key=%d, addr=%p): %m", (int) UsedShmemSegID, UsedShmemSegAddr); if (hdr != origUsedShmemSegAddr) elog(FATAL, "reattaching to shared memory returned unexpected address (got %p, expected %p)", hdr, origUsedShmemSegAddr); - dsm_set_control_handle(hdr->dsm_control); + dsm_set_control_handle(((PGShmemHeader *) hdr)->dsm_control); UsedShmemSegAddr = hdr; /* probably redundant */ } @@ -876,3 +811,31 @@ PGSharedMemoryDetach(void) } #endif } + + +/* + * Attach to shared memory and make sure it has a Postgres header + * + * Returns attach address if OK, else NULL + */ +static PGShmemHeader * +PGSharedMemoryAttach(IpcMemoryKey key, IpcMemoryId *shmid) +{ + PGShmemHeader *hdr; + + if ((*shmid = shmget(key, sizeof(PGShmemHeader), 0)) < 0) + return NULL; + + hdr = (PGShmemHeader *) shmat(*shmid, UsedShmemSegAddr, PG_SHMAT_FLAGS); + + if (hdr == (PGShmemHeader *) -1) + return NULL; /* failed: must be some other app's */ + + if (hdr->magic != PGShmemMagic) + { + shmdt((void *) hdr); + return NULL; /* segment belongs to a non-Postgres app */ + } + + return hdr; +} diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.c index 3888c26db5..f8ca52e1af 100644 --- a/src/backend/port/win32_shmem.c +++ b/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; diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 0f74f243c5..2215ebbb5a 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -2509,7 +2509,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); } @@ -4877,7 +4877,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 */ @@ -4891,7 +4891,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 */ } @@ -4904,7 +4904,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 */ } @@ -4917,7 +4917,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 */ } @@ -4935,7 +4935,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); diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c index 091244dde8..0c86a581c0 100644 --- a/src/backend/storage/ipc/ipci.c +++ b/src/backend/storage/ipc/ipci.c @@ -88,9 +88,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 } diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 9676fda866..09e0df290d 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -422,11 +422,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); } } diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.h index 02f4b1ba79..6a05a89349 100644 --- a/src/include/storage/ipc.h +++ b/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 */ diff --git a/src/include/storage/pg_shmem.h b/src/include/storage/pg_shmem.h index 0a6519650e..6b1e040251 100644 --- a/src/include/storage/pg_shmem.h +++ b/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 */ @@ -64,8 +64,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); diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 0335ab22f2..044b07790c 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -104,8 +104,7 @@ our @EXPORT = qw( get_new_node ); -our ($use_tcp, $test_localhost, $test_pghost, $last_host_assigned, - $last_port_assigned, @all_nodes, $died); +our ($test_localhost, $test_pghost, $last_port_assigned, @all_nodes, $died); # Windows path to virtual file system root @@ -119,14 +118,13 @@ if ($Config{osname} eq 'msys') 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 = $TestLib::windows_os; - $test_localhost = "127.0.0.1"; - $last_host_assigned = 1; - $test_pghost = $use_tcp ? $test_localhost : TestLib::tempdir_short; - $ENV{PGHOST} = $test_pghost; - $ENV{PGDATABASE} = 'postgres'; + # 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 ? $test_localhost : TestLib::tempdir_short; + $ENV{PGHOST} = $test_pghost; + $ENV{PGDATABASE} = 'postgres'; # Tracking of last port value assigned to accelerate free port lookup. $last_port_assigned = int(rand() * 16384) + 49152; @@ -157,9 +155,7 @@ sub new _host => $pghost, _basedir => "$TestLib::tmp_check/t_${testname}_${name}_data", _name => $name, - _logfile_generation => 0, - _logfile_base => "$TestLib::log_path/${testname}_${name}", - _logfile => "$TestLib::log_path/${testname}_${name}.log" + _logfile => "$TestLib::log_path/${testname}_${name}.log" }; bless $self, $class; @@ -477,9 +473,8 @@ sub init print $conf "max_wal_senders = 0\n"; } - if ($use_tcp) + if ($TestLib::windows_os) { - print $conf "unix_socket_directories = ''\n"; print $conf "listen_addresses = '$host'\n"; } else @@ -541,11 +536,12 @@ sub backup { my ($self, $backup_name) = @_; my $backup_path = $self->backup_dir . '/' . $backup_name; + my $port = $self->port; my $name = $self->name; print "# Taking pg_basebackup $backup_name from node \"$name\"\n"; - TestLib::system_or_bail('pg_basebackup', '-D', $backup_path, '-h', - $self->host, '-p', $self->port, '--no-sync'); + TestLib::system_or_bail('pg_basebackup', '-D', $backup_path, '-p', $port, + '--no-sync'); print "# Backup finished\n"; return; } @@ -657,7 +653,6 @@ sub init_from_backup { my ($self, $root_node, $backup_name, %params) = @_; my $backup_path = $root_node->backup_dir . '/' . $backup_name; - my $host = $self->host; my $port = $self->port; my $node_name = $self->name; my $root_name = $root_node->name; @@ -684,15 +679,6 @@ 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->enable_streaming($root_node) if $params{has_streaming}; $self->enable_restoring($root_node) if $params{has_restoring}; return; @@ -700,45 +686,17 @@ port = $port =pod -=item $node->rotate_logfile() - -Switch to a new PostgreSQL log file. This does not alter any running -PostgreSQL process. Subsequent method calls, including pg_ctl invocations, -will use the new name. Return the new name. - -=cut - -sub rotate_logfile -{ - my ($self) = @_; - $self->{_logfile} = sprintf('%s_%d.log', - $self->{_logfile_base}, - ++$self->{_logfile_generation}); - return $self->{_logfile}; -} - -=pod - -=item $node->start(%params) => success_or_failure +=item $node->start() Wrapper for pg_ctl start Start the node and wait until it is ready to accept connections. -=over - -=item fail_ok => 1 - -By default, failure terminates the entire F invocation. If given, -instead return a true or false value to indicate success or failure. - -=back - =cut sub start { - my ($self, %params) = @_; + my ($self) = @_; my $port = $self->port; my $pgdata = $self->data_dir; my $name = $self->name; @@ -751,34 +709,10 @@ sub start { print "# pg_ctl start failed; logfile:\n"; print TestLib::slurp_file($self->logfile); - BAIL_OUT("pg_ctl start failed") unless $params{fail_ok}; - return 0; + BAIL_OUT("pg_ctl start failed"); } $self->_update_pid(1); - return 1; -} - -=pod - -=item $node->kill9() - -Send SIGKILL (signal 9) to the postmaster. - -Note: if the node is already known stopped, this does nothing. -However, if we think it's running and it's not, it's important for -this to fail. Otherwise, tests might fail to detect server crashes. - -=cut - -sub kill9 -{ - my ($self) = @_; - my $name = $self->name; - return unless defined $self->{_pid}; - print "### Killing node \"$name\" using signal 9\n"; - kill(9, $self->{_pid}) or BAIL_OUT("kill(9, $self->{_pid}) failed"); - $self->{_pid} = undef; return; } @@ -974,7 +908,7 @@ sub _update_pid =pod -=item PostgresNode->get_new_node(node_name, %params) +=item PostgresNode->get_new_node(node_name) Build a new object of class C (or of a subclass, if you have one), assigning a free port number. Remembers the node, to prevent its port @@ -983,22 +917,6 @@ shut down when the test script exits. You should generally use this instead of C. -=over - -=item port => [1,65535] - -By default, this function assigns a port number to each node. Specify this to -force a particular port number. The caller is responsible for evaluating -potential conflicts and privilege requirements. - -=item own_host => 1 - -By default, all nodes use the same PGHOST value. If specified, generate a -PGHOST specific to this node. This allows multiple nodes to use the same -port. - -=back - For backwards compatibility, it is also exported as a standalone function, which can only create objects of class C. @@ -1007,11 +925,10 @@ which can only create objects of class C. sub get_new_node { my $class = 'PostgresNode'; - $class = shift if scalar(@_) % 2 != 1; - my ($name, %params) = @_; - my $port_is_forced = defined $params{port}; - my $found = $port_is_forced; - my $port = $port_is_forced ? $params{port} : $last_port_assigned; + $class = shift if 1 < scalar @_; + my $name = shift; + my $found = 0; + my $port = $last_port_assigned; while ($found == 0) { @@ -1028,15 +945,13 @@ sub get_new_node $found = 0 if ($node->port == $port); } - # Check to see if anything else is listening on this TCP port. Accept - # only ports available for all possible listen_addresses values, so - # the caller can harness this port for the widest range of purposes. - # 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. + # 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('0.0.0.0'); + my $iaddr = inet_aton($test_localhost); my $paddr = sockaddr_in($port, $iaddr); my $proto = getprotobyname("tcp"); @@ -1052,35 +967,16 @@ sub get_new_node } } - print "# Found port $port\n"; - - # Select a host. - my $host = $test_pghost; - if ($params{own_host}) - { - if ($use_tcp) - { - # This assumes $use_tcp platforms treat every address in - # 127.0.0.1/24, not just 127.0.0.1, as a usable loopback. - $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; - } - } + print "# Found free port $port\n"; # Lock port number found by creating a new node - my $node = $class->new($name, $host, $port); + my $node = $class->new($name, $test_pghost, $port); # Add node to list of nodes push(@all_nodes, $node); # And update port for next time - $port_is_forced or $last_port_assigned = $port; + $last_port_assigned = $port; return $node; } @@ -1471,8 +1367,9 @@ $stderr); =item $node->command_ok(...) -Runs a shell command like TestLib::command_ok, but with PGHOST and PGPORT set -so that the command will default to connecting to this PostgresNode. +Runs a shell command like TestLib::command_ok, but with PGPORT +set so that the command will default to connecting to this +PostgresNode. =cut @@ -1480,7 +1377,6 @@ sub command_ok { my $self = shift; - local $ENV{PGHOST} = $self->host; local $ENV{PGPORT} = $self->port; TestLib::command_ok(@_); @@ -1491,7 +1387,7 @@ sub command_ok =item $node->command_fails(...) -TestLib::command_fails with our connection parameters. See command_ok(...) +TestLib::command_fails with our PGPORT. See command_ok(...) =cut @@ -1499,7 +1395,6 @@ sub command_fails { my $self = shift; - local $ENV{PGHOST} = $self->host; local $ENV{PGPORT} = $self->port; TestLib::command_fails(@_); @@ -1510,7 +1405,7 @@ sub command_fails =item $node->command_like(...) -TestLib::command_like with our connection parameters. See command_ok(...) +TestLib::command_like with our PGPORT. See command_ok(...) =cut @@ -1518,7 +1413,6 @@ sub command_like { my $self = shift; - local $ENV{PGHOST} = $self->host; local $ENV{PGPORT} = $self->port; TestLib::command_like(@_); @@ -1529,8 +1423,7 @@ sub command_like =item $node->command_checks_all(...) -TestLib::command_checks_all with our connection parameters. See -command_ok(...) +TestLib::command_checks_all with our PGPORT. See command_ok(...) =cut @@ -1538,7 +1431,6 @@ sub command_checks_all { my $self = shift; - local $ENV{PGHOST} = $self->host; local $ENV{PGPORT} = $self->port; TestLib::command_checks_all(@_); @@ -1561,7 +1453,6 @@ sub issues_sql_like { my ($self, $cmd, $expected_sql, $test_name) = @_; - local $ENV{PGHOST} = $self->host; local $ENV{PGPORT} = $self->port; truncate $self->logfile, 0; @@ -1576,8 +1467,8 @@ sub issues_sql_like =item $node->run_log(...) -Runs a shell command like TestLib::run_log, but with connection parameters set -so that the command will default to connecting to this PostgresNode. +Runs a shell command like TestLib::run_log, but with PGPORT set so +that the command will default to connecting to this PostgresNode. =cut @@ -1585,7 +1476,6 @@ sub run_log { my $self = shift; - local $ENV{PGHOST} = $self->host; local $ENV{PGPORT} = $self->port; TestLib::run_log(@_); diff --git a/src/test/recovery/t/017_shm.pl b/src/test/recovery/t/017_shm.pl deleted file mode 100644 index 8e85d6b7cd..0000000000 --- a/src/test/recovery/t/017_shm.pl +++ /dev/null @@ -1,177 +0,0 @@ -# -# 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', <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; -} diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl index 3e3a058794..24ac6a5e4d 100644 --- a/src/tools/msvc/vcregress.pl +++ b/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";