diff --git a/src/backend/port/posix_sema.c b/src/backend/port/posix_sema.c index 3370adf..bdd6552 100644 --- a/src/backend/port/posix_sema.c +++ b/src/backend/port/posix_sema.c @@ -29,6 +29,7 @@ #include #include #include +#include #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 ac5106f..a1652cc 100644 --- a/src/backend/port/sysv_sema.c +++ b/src/backend/port/sysv_sema.c @@ -17,6 +17,7 @@ #include #include #include +#include #ifdef HAVE_SYS_IPC_H #include #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 968506d..dab2920 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 013c122..32cc697 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 ccd7b6b..6cb6328 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 3339804..67cdeb5 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(); } @@ -3919,7 +3918,7 @@ PostmasterStateMachine(void) /* re-read control file into local memory */ LocalProcessControlFile(true); - reset_shared(PostPortNumber); + reset_shared(); StartupPID = StartupDataBase(); Assert(StartupPID != 0); @@ -4947,7 +4946,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 */ @@ -4961,7 +4960,7 @@ SubPostmasterMain(int argc, char *argv[]) InitAuxiliaryProcess(); /* Attach process to shared data structures */ - CreateSharedMemoryAndSemaphores(0); + CreateSharedMemoryAndSemaphores(); AuxiliaryProcessMain(argc - 2, argv + 2); /* does not return */ } @@ -4974,7 +4973,7 @@ SubPostmasterMain(int argc, char *argv[]) InitProcess(); /* Attach process to shared data structures */ - CreateSharedMemoryAndSemaphores(0); + CreateSharedMemoryAndSemaphores(); AutoVacLauncherMain(argc - 2, argv + 2); /* does not return */ } @@ -4987,7 +4986,7 @@ SubPostmasterMain(int argc, char *argv[]) InitProcess(); /* Attach process to shared data structures */ - CreateSharedMemoryAndSemaphores(0); + CreateSharedMemoryAndSemaphores(); AutoVacWorkerMain(argc - 2, argv + 2); /* does not return */ } @@ -5005,7 +5004,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 d7d7335..8853706 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 43b9f17..29c5ec7 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 e9b243f..d113813 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 b95efa1..1ac4f9a 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 ac24878..6bd5664 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 7f10ff5..0b988eb 100644 --- a/src/test/recovery/t/017_shm.pl +++ b/src/test/recovery/t/017_shm.pl @@ -152,14 +152,12 @@ 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. +# Reject startup if shm key N has become available and we crash while +# using key N+1. $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) + 0, + 'key turnover fools nobody'); $flea->start; # grab first key # cleanup TestLib::system_log('pg_ctl', 'kill', 'QUIT', $slow_pid);