diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/backend/port/posix_sema.c | 23 | ||||
| -rw-r--r-- | src/backend/port/sysv_sema.c | 23 | ||||
| -rw-r--r-- | src/backend/port/sysv_shmem.c | 38 | ||||
| -rw-r--r-- | src/backend/port/win32_sema.c | 2 | ||||
| -rw-r--r-- | src/backend/port/win32_shmem.c | 2 | ||||
| -rw-r--r-- | src/backend/postmaster/postmaster.c | 25 | ||||
| -rw-r--r-- | src/backend/storage/ipc/ipci.c | 6 | ||||
| -rw-r--r-- | src/backend/utils/init/postinit.c | 8 | ||||
| -rw-r--r-- | src/include/storage/ipc.h | 2 | ||||
| -rw-r--r-- | src/include/storage/pg_sema.h | 2 | ||||
| -rw-r--r-- | src/include/storage/pg_shmem.h | 2 | ||||
| -rw-r--r-- | src/test/recovery/t/017_shm.pl | 150 |
12 files changed, 159 insertions, 124 deletions
diff --git a/src/backend/port/posix_sema.c b/src/backend/port/posix_sema.c index 3370adf35e2..bdd6552f614 100644 --- a/src/backend/port/posix_sema.c +++ b/src/backend/port/posix_sema.c @@ -29,6 +29,7 @@ #include <semaphore.h> #include <signal.h> #include <unistd.h> +#include <sys/stat.h> #include "miscadmin.h" #include "storage/ipc.h" @@ -181,10 +182,6 @@ PGSemaphoreShmemSize(int maxSemas) * are acquired here or in PGSemaphoreCreate, register an on_shmem_exit * callback to release them. * - * The port number is passed for possible use as a key (for Posix, we use - * it to generate the starting semaphore name). In a standalone backend, - * zero will be passed. - * * In the Posix implementation, we acquire semaphores on-demand; the * maxSemas parameter is just used to size the arrays. For unnamed * semaphores, there is an array of PGSemaphoreData structs in shared memory. @@ -196,8 +193,22 @@ PGSemaphoreShmemSize(int maxSemas) * we don't have to expose the counters to other processes.) */ void -PGReserveSemaphores(int maxSemas, int port) +PGReserveSemaphores(int maxSemas) { + struct stat statbuf; + + /* + * We use the data directory's inode number to seed the search for free + * semaphore keys. This minimizes the odds of collision with other + * postmasters, while maximizing the odds that we will detect and clean up + * semaphores left over from a crashed postmaster in our own directory. + */ + if (stat(DataDir, &statbuf) < 0) + ereport(FATAL, + (errcode_for_file_access(), + errmsg("could not stat data directory \"%s\": %m", + DataDir))); + #ifdef USE_NAMED_POSIX_SEMAPHORES mySemPointers = (sem_t **) malloc(maxSemas * sizeof(sem_t *)); if (mySemPointers == NULL) @@ -215,7 +226,7 @@ PGReserveSemaphores(int maxSemas, int port) numSems = 0; maxSems = maxSemas; - nextSemKey = port * 1000; + nextSemKey = statbuf.st_ino; on_shmem_exit(ReleaseSemaphores, 0); } diff --git a/src/backend/port/sysv_sema.c b/src/backend/port/sysv_sema.c index ac5106f39da..a1652cce59c 100644 --- a/src/backend/port/sysv_sema.c +++ b/src/backend/port/sysv_sema.c @@ -17,6 +17,7 @@ #include <signal.h> #include <unistd.h> #include <sys/file.h> +#include <sys/stat.h> #ifdef HAVE_SYS_IPC_H #include <sys/ipc.h> #endif @@ -301,10 +302,6 @@ PGSemaphoreShmemSize(int maxSemas) * are acquired here or in PGSemaphoreCreate, register an on_shmem_exit * callback to release them. * - * The port number is passed for possible use as a key (for SysV, we use - * it to generate the starting semaphore key). In a standalone backend, - * zero will be passed. - * * In the SysV implementation, we acquire semaphore sets on-demand; the * maxSemas parameter is just used to size the arrays. There is an array * of PGSemaphoreData structs in shared memory, and a postmaster-local array @@ -314,8 +311,22 @@ PGSemaphoreShmemSize(int maxSemas) * have clobbered.) */ void -PGReserveSemaphores(int maxSemas, int port) +PGReserveSemaphores(int maxSemas) { + struct stat statbuf; + + /* + * We use the data directory's inode number to seed the search for free + * semaphore keys. This minimizes the odds of collision with other + * postmasters, while maximizing the odds that we will detect and clean up + * semaphores left over from a crashed postmaster in our own directory. + */ + if (stat(DataDir, &statbuf) < 0) + ereport(FATAL, + (errcode_for_file_access(), + errmsg("could not stat data directory \"%s\": %m", + DataDir))); + /* * We must use ShmemAllocUnlocked(), since the spinlock protecting * ShmemAlloc() won't be ready yet. (This ordering is necessary when we @@ -332,7 +343,7 @@ PGReserveSemaphores(int maxSemas, int port) if (mySemaSets == NULL) elog(PANIC, "out of memory"); numSemaSets = 0; - nextSemaKey = port * 1000; + nextSemaKey = statbuf.st_ino; nextSemaNumber = SEMAS_PER_SET; /* force sema set alloc on 1st call */ on_shmem_exit(ReleaseSemaphores, 0); diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c index 968506dd516..dab2920ad61 100644 --- a/src/backend/port/sysv_shmem.c +++ b/src/backend/port/sysv_shmem.c @@ -390,11 +390,12 @@ PGSharedMemoryAttach(IpcMemoryId shmId, /* * Try to attach to the segment and see if it matches our data directory. - * This avoids key-conflict problems on machines that are running several - * postmasters under the same userid and port number. (That would not - * ordinarily happen in production, but it can happen during parallel - * testing. Since our test setups don't open any TCP ports on Unix, such - * cases don't conflict otherwise.) + * This avoids any risk of duplicate-shmem-key conflicts on machines that + * are running several postmasters under the same userid. + * + * (When we're called from PGSharedMemoryCreate, this stat call is + * duplicative; but since this isn't a high-traffic case it's not worth + * trying to optimize.) */ if (stat(DataDir, &statbuf) < 0) return SHMSTATE_ANALYSIS_FAILURE; /* can't stat; be conservative */ @@ -617,12 +618,9 @@ AnonymousShmemDetach(int status, Datum arg) * 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. - * - * The port number is passed for possible use as a key (for SysV, we use - * it to generate the starting shmem key). */ PGShmemHeader * -PGSharedMemoryCreate(Size size, int port, +PGSharedMemoryCreate(Size size, PGShmemHeader **shim) { IpcMemoryKey NextShmemSegID; @@ -631,6 +629,17 @@ PGSharedMemoryCreate(Size size, int port, struct stat statbuf; Size sysvsize; + /* + * We use the data directory's ID info (inode and device numbers) to + * positively identify shmem segments associated with this data dir, and + * also as seeds for searching for a free shmem key. + */ + if (stat(DataDir, &statbuf) < 0) + ereport(FATAL, + (errcode_for_file_access(), + errmsg("could not stat data directory \"%s\": %m", + DataDir))); + /* Complain if hugepages demanded but we can't possibly support them */ #if !defined(MAP_HUGETLB) if (huge_pages == HUGE_PAGES_ON) @@ -659,10 +668,10 @@ PGSharedMemoryCreate(Size size, int port, /* * 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.) + * loop simultaneously. (CreateDataDirLockFile() does not entirely ensure + * that, but prefer fixing it over coping here.) */ - NextShmemSegID = 1 + port * 1000; + NextShmemSegID = statbuf.st_ino; for (;;) { @@ -748,11 +757,6 @@ PGSharedMemoryCreate(Size size, int port, hdr->dsm_control = 0; /* Fill in the data directory ID info, too */ - if (stat(DataDir, &statbuf) < 0) - ereport(FATAL, - (errcode_for_file_access(), - errmsg("could not stat data directory \"%s\": %m", - DataDir))); hdr->device = statbuf.st_dev; hdr->inode = statbuf.st_ino; diff --git a/src/backend/port/win32_sema.c b/src/backend/port/win32_sema.c index 013c122451d..32cc6978669 100644 --- a/src/backend/port/win32_sema.c +++ b/src/backend/port/win32_sema.c @@ -44,7 +44,7 @@ PGSemaphoreShmemSize(int maxSemas) * process exits. */ void -PGReserveSemaphores(int maxSemas, int port) +PGReserveSemaphores(int maxSemas) { mySemSet = (HANDLE *) malloc(maxSemas * sizeof(HANDLE)); if (mySemSet == NULL) diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.c index ccd7b6b5e33..6cb63282847 100644 --- a/src/backend/port/win32_shmem.c +++ b/src/backend/port/win32_shmem.c @@ -194,7 +194,7 @@ EnableLockPagesPrivilege(int elevel) * standard header. */ PGShmemHeader * -PGSharedMemoryCreate(Size size, int port, +PGSharedMemoryCreate(Size size, PGShmemHeader **shim) { void *memAddress; diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 62dc93d56ba..a5446d54bb5 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -385,7 +385,7 @@ static void getInstallationPaths(const char *argv0); static void checkControlFile(void); static Port *ConnCreate(int serverFd); static void ConnFree(Port *port); -static void reset_shared(int port); +static void reset_shared(void); static void SIGHUP_handler(SIGNAL_ARGS); static void pmdie(SIGNAL_ARGS); static void reaper(SIGNAL_ARGS); @@ -1175,7 +1175,7 @@ PostmasterMain(int argc, char *argv[]) /* * Set up shared memory and semaphores. */ - reset_shared(PostPortNumber); + reset_shared(); /* * Estimate number of openable files. This must happen after setting up @@ -2599,17 +2599,16 @@ InitProcessGlobals(void) * reset_shared -- reset shared memory and semaphores */ static void -reset_shared(int port) +reset_shared(void) { /* * Create or re-create shared memory and semaphores. * * Note: in each "cycle of life" we will normally assign the same IPC keys - * (if using SysV shmem and/or semas), since the port number is used to - * determine IPC keys. This helps ensure that we will clean up dead IPC - * objects if the postmaster crashes and is restarted. + * (if using SysV shmem and/or semas). This helps ensure that we will + * clean up dead IPC objects if the postmaster crashes and is restarted. */ - CreateSharedMemoryAndSemaphores(port); + CreateSharedMemoryAndSemaphores(); } @@ -3934,7 +3933,7 @@ PostmasterStateMachine(void) /* re-read control file into local memory */ LocalProcessControlFile(true); - reset_shared(PostPortNumber); + reset_shared(); StartupPID = StartupDataBase(); Assert(StartupPID != 0); @@ -4962,7 +4961,7 @@ SubPostmasterMain(int argc, char *argv[]) InitProcess(); /* Attach process to shared data structures */ - CreateSharedMemoryAndSemaphores(0); + CreateSharedMemoryAndSemaphores(); /* And run the backend */ BackendRun(&port); /* does not return */ @@ -4976,7 +4975,7 @@ SubPostmasterMain(int argc, char *argv[]) InitAuxiliaryProcess(); /* Attach process to shared data structures */ - CreateSharedMemoryAndSemaphores(0); + CreateSharedMemoryAndSemaphores(); AuxiliaryProcessMain(argc - 2, argv + 2); /* does not return */ } @@ -4989,7 +4988,7 @@ SubPostmasterMain(int argc, char *argv[]) InitProcess(); /* Attach process to shared data structures */ - CreateSharedMemoryAndSemaphores(0); + CreateSharedMemoryAndSemaphores(); AutoVacLauncherMain(argc - 2, argv + 2); /* does not return */ } @@ -5002,7 +5001,7 @@ SubPostmasterMain(int argc, char *argv[]) InitProcess(); /* Attach process to shared data structures */ - CreateSharedMemoryAndSemaphores(0); + CreateSharedMemoryAndSemaphores(); AutoVacWorkerMain(argc - 2, argv + 2); /* does not return */ } @@ -5020,7 +5019,7 @@ SubPostmasterMain(int argc, char *argv[]) InitProcess(); /* Attach process to shared data structures */ - CreateSharedMemoryAndSemaphores(0); + CreateSharedMemoryAndSemaphores(); /* 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 d7d733530ff..885370698fc 100644 --- a/src/backend/storage/ipc/ipci.c +++ b/src/backend/storage/ipc/ipci.c @@ -91,7 +91,7 @@ RequestAddinShmemSpace(Size size) * This is a bit code-wasteful and could be cleaned up.) */ void -CreateSharedMemoryAndSemaphores(int port) +CreateSharedMemoryAndSemaphores(void) { PGShmemHeader *shim = NULL; @@ -163,14 +163,14 @@ CreateSharedMemoryAndSemaphores(int port) /* * Create the shmem segment */ - seghdr = PGSharedMemoryCreate(size, port, &shim); + seghdr = PGSharedMemoryCreate(size, &shim); InitShmemAccess(seghdr); /* * Create semaphores */ - PGReserveSemaphores(numSemas, port); + PGReserveSemaphores(numSemas); /* * If spinlocks are disabled, initialize emulation layer (which diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 43b9f17f722..29c5ec7b58b 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -445,12 +445,10 @@ InitCommunication(void) if (!IsUnderPostmaster) /* postmaster already did this */ { /* - * 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. + * We're running a postgres bootstrap process or a standalone backend, + * so we need to set up shmem. */ - CreateSharedMemoryAndSemaphores(PostPortNumber); + CreateSharedMemoryAndSemaphores(); } } diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.h index e9b243fe7d4..d113813df32 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(void); #endif /* IPC_H */ diff --git a/src/include/storage/pg_sema.h b/src/include/storage/pg_sema.h index b95efa1e22e..1ac4f9a830f 100644 --- a/src/include/storage/pg_sema.h +++ b/src/include/storage/pg_sema.h @@ -41,7 +41,7 @@ typedef HANDLE PGSemaphore; extern Size PGSemaphoreShmemSize(int maxSemas); /* Module initialization (called during postmaster start or shmem reinit) */ -extern void PGReserveSemaphores(int maxSemas, int port); +extern void PGReserveSemaphores(int maxSemas); /* Allocate a PGSemaphore structure with initial count 1 */ extern PGSemaphore PGSemaphoreCreate(void); diff --git a/src/include/storage/pg_shmem.h b/src/include/storage/pg_shmem.h index ac24878efbf..6bd566405b2 100644 --- a/src/include/storage/pg_shmem.h +++ b/src/include/storage/pg_shmem.h @@ -82,7 +82,7 @@ extern void PGSharedMemoryReAttach(void); extern void PGSharedMemoryNoReAttach(void); #endif -extern PGShmemHeader *PGSharedMemoryCreate(Size size, int port, +extern PGShmemHeader *PGSharedMemoryCreate(Size size, PGShmemHeader **shim); extern bool PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2); extern void PGSharedMemoryDetach(void); diff --git a/src/test/recovery/t/017_shm.pl b/src/test/recovery/t/017_shm.pl index 7f10ff5225b..a29ef788551 100644 --- a/src/test/recovery/t/017_shm.pl +++ b/src/test/recovery/t/017_shm.pl @@ -4,16 +4,30 @@ use strict; use warnings; use Config; +use File::stat qw(stat); use IPC::Run 'run'; use PostgresNode; use Test::More; use TestLib; use Time::HiRes qw(usleep); -plan tests => 5; +# If we don't have shmem support, skip the whole thing +eval { + require IPC::SharedMem; + IPC::SharedMem->import; + require IPC::SysV; + IPC::SysV->import(qw(IPC_CREAT IPC_EXCL S_IRUSR S_IWUSR)); +}; +if ($@) +{ + plan skip_all => 'SysV shared memory not supported by this platform'; +} +else +{ + plan tests => 4; +} my $tempdir = TestLib::tempdir; -my $port; # Log "ipcs" diffs on a best-effort basis, swallowing any error. my $ipcs_before = "$tempdir/ipcs_before"; @@ -25,76 +39,80 @@ sub log_ipcs return; } -# These tests need a $port such that nothing creates or removes a segment in -# $port's IpcMemoryKey range while this test script runs. While there's no -# way to ensure that in general, we do ensure that if PostgreSQL tests are the -# only actors. With TCP, the first get_new_node picks a port number. With -# Unix sockets, use a postmaster, $port_holder, to represent a key space -# reservation. $port_holder holds a reservation on the key space of port -# 1+$port_holder->port if it created the first IpcMemoryKey of its own port's -# key space. If multiple copies of this test script run concurrently, they -# will pick different ports. $port_holder postmasters use odd-numbered ports, -# and tests use even-numbered ports. In the absence of collisions from other -# shmget() activity, gnat starts with key 0x7d001 (512001), and flea starts -# with key 0x7d002 (512002). -my $port_holder; -if (!$PostgresNode::use_tcp) -{ - my $lock_port; - for ($lock_port = 511; $lock_port < 711; $lock_port += 2) - { - $port_holder = PostgresNode->get_new_node( - "port${lock_port}_holder", - port => $lock_port, - own_host => 1); - $port_holder->init; - $port_holder->append_conf('postgresql.conf', 'max_connections = 5'); - $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 + $lock_port * 1000); - last - if slurp_file($port_holder->data_dir . '/postmaster.pid') =~ - /^$shmem_key_line_prefix/m; - $port_holder->stop; - } - $port = $lock_port + 1; -} - # 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; - # Limit semaphore consumption, since we run several nodes concurrently. - $ret->append_conf('postgresql.conf', 'max_connections = 5'); - $ret->start; - log_ipcs(); - return $ret; -} -my $gnat = init_start 'gnat'; -my $flea = init_start 'flea'; +my $gnat = PostgresNode->get_new_node('gnat'); +$gnat->init; + +# Create a shmem segment that will conflict with gnat's first choice +# of shmem key. (If we fail to create it because something else is +# already using that key, that's perfectly fine, though the test will +# exercise a different scenario than it usually does.) +my $gnat_dir_stat = stat($gnat->data_dir); +defined($gnat_dir_stat) or die('unable to stat ' . $gnat->data_dir); +my $gnat_inode = $gnat_dir_stat->ino; +note "gnat's datadir inode = $gnat_inode"; + +# Note: must reference IPC::SysV's constants as functions, or this file +# fails to compile when that module is not available. +my $gnat_conflict_shm = + IPC::SharedMem->new($gnat_inode, 1024, + IPC_CREAT() | IPC_EXCL() | S_IRUSR() | S_IWUSR()); +note "could not create conflicting shmem" if !defined($gnat_conflict_shm); +log_ipcs(); + +$gnat->start; +log_ipcs(); + +$gnat->restart; # should keep same shmem key +log_ipcs(); # 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. +note "removing the conflicting shmem ..."; +$gnat_conflict_shm->remove if $gnat_conflict_shm; +log_ipcs(); + +# Upon postmaster death, postmaster children exit automatically. +$gnat->kill9; +log_ipcs(); + +# In this start, gnat will use its normal shmem key, and fail to remove +# the higher-keyed segment that the previous postmaster was using. +# That's not great, but key collisions should be rare enough to not +# make this a big problem. +poll_start($gnat); +log_ipcs(); $gnat->stop; -$flea->restart; log_ipcs(); + +# Re-create the conflicting segment, and start/stop normally, just so +# this test script doesn't leak the higher-keyed segment. +note "re-creating conflicting shmem ..."; +$gnat_conflict_shm = + IPC::SharedMem->new($gnat_inode, 1024, + IPC_CREAT() | IPC_EXCL() | S_IRUSR() | S_IWUSR()); +note "could not create conflicting shmem" if !defined($gnat_conflict_shm); +log_ipcs(); + $gnat->start; log_ipcs(); +$gnat->stop; +log_ipcs(); + +note "removing the conflicting shmem ..."; +$gnat_conflict_shm->remove if $gnat_conflict_shm; +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->start; +log_ipcs(); + my $regress_shlib = TestLib::perl2host($ENV{REGRESS_SHLIB}); $gnat->safe_psql('postgres', <<EOSQL); CREATE FUNCTION wait_pid(int) @@ -152,24 +170,18 @@ print STDERR $single_stderr; like($single_stderr, $pre_existing_msg, '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 + +# cleanup slow backend 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 +# now startup should work +poll_start($gnat); +log_ipcs(); + +# finish testing $gnat->stop; -$flea->stop; -$port_holder->stop if $port_holder; log_ipcs(); |