diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c index 0d06747298c..fa454e1927d 100644 --- a/src/backend/port/sysv_shmem.c +++ b/src/backend/port/sysv_shmem.c @@ -103,6 +103,7 @@ 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, + void *attachAt, PGShmemHeader **addr); @@ -310,7 +311,7 @@ PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2) PGShmemHeader *memAddress; IpcMemoryState state; - state = PGSharedMemoryAttach((IpcMemoryId) id2, &memAddress); + state = PGSharedMemoryAttach((IpcMemoryId) id2, NULL, &memAddress); if (memAddress && shmdt(memAddress) < 0) elog(LOG, "shmdt(%p) failed: %m", memAddress); switch (state) @@ -326,9 +327,17 @@ PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2) return true; } -/* See comment at IpcMemoryState. */ +/* + * Test for a segment with id shmId; see comment at IpcMemoryState. + * + * If the segment exists, we'll attempt to attach to it, using attachAt + * if that's not NULL (but it's best to pass NULL if possible). + * + * *addr is set to the segment memory address if we attached to it, else NULL. + */ static IpcMemoryState PGSharedMemoryAttach(IpcMemoryId shmId, + void *attachAt, PGShmemHeader **addr) { struct shmid_ds shmStat; @@ -338,8 +347,7 @@ PGSharedMemoryAttach(IpcMemoryId shmId, *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. + * First, try to stat the shm segment ID, to see if it exists at all. */ if (shmctl(shmId, IPC_STAT, &shmStat) < 0) { @@ -372,9 +380,10 @@ PGSharedMemoryAttach(IpcMemoryId shmId, #endif /* - * Otherwise, we had better assume that the segment is in use. The - * only likely case is EIDRM, which implies that the segment has been - * IPC_RMID'd but there are still processes attached to it. + * Otherwise, we had better assume that the segment is in use. The + * only likely case is (non-Linux, assumed spec-compliant) EIDRM, + * which implies that the segment has been IPC_RMID'd but there are + * still processes attached to it. */ return SHMSTATE_ANALYSIS_FAILURE; } @@ -382,24 +391,37 @@ PGSharedMemoryAttach(IpcMemoryId shmId, /* * 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. + * 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.) */ if (stat(DataDir, &statbuf) < 0) return SHMSTATE_ANALYSIS_FAILURE; /* can't stat; be conservative */ - /* - * Attachment fails if we have no write permission. Since that will never - * happen with Postgres IPCProtection, such a failure shows the segment is - * not a Postgres segment. If attachment fails for some other reason, be - * conservative. - */ - hdr = (PGShmemHeader *) shmat(shmId, UsedShmemSegAddr, PG_SHMAT_FLAGS); + hdr = (PGShmemHeader *) shmat(shmId, attachAt, PG_SHMAT_FLAGS); if (hdr == (PGShmemHeader *) -1) { + /* + * Attachment failed. The cases we're interested in are the same as + * for the shmctl() call above. In particular, note that the owning + * postmaster could have terminated and removed the segment between + * shmctl() and shmat(). + * + * If attachAt isn't NULL, it's possible that EINVAL reflects a + * problem with that address not a vanished segment, so it's best to + * pass NULL when probing for conflicting segments. + */ + if (errno == EINVAL) + return SHMSTATE_ENOENT; /* segment disappeared */ if (errno == EACCES) - return SHMSTATE_FOREIGN; - else - return SHMSTATE_ANALYSIS_FAILURE; + return SHMSTATE_FOREIGN; /* must be non-Postgres */ +#ifdef HAVE_LINUX_EIDRM_BUG + if (errno == EIDRM) + return SHMSTATE_ENOENT; /* segment disappeared */ +#endif + /* Otherwise, be conservative. */ + return SHMSTATE_ANALYSIS_FAILURE; } *addr = hdr; @@ -414,6 +436,11 @@ PGSharedMemoryAttach(IpcMemoryId shmId, return SHMSTATE_FOREIGN; } + /* + * It does match our data directory, so now test whether any processes are + * still attached to it. (We are, now, but the shm_nattch result is from + * before we attached to it.) + */ return shmStat.shm_nattch == 0 ? SHMSTATE_UNATTACHED : SHMSTATE_ATTACHED; } @@ -632,9 +659,6 @@ PGSharedMemoryCreate(Size size, int port, sysvsize = size; #endif - /* 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 @@ -668,7 +692,7 @@ PGSharedMemoryCreate(Size size, int port, state = SHMSTATE_FOREIGN; } else - state = PGSharedMemoryAttach(shmid, &oldhdr); + state = PGSharedMemoryAttach(shmid, NULL, &oldhdr); switch (state) { @@ -798,7 +822,7 @@ PGSharedMemoryReAttach(void) if (shmid < 0) state = SHMSTATE_FOREIGN; else - state = PGSharedMemoryAttach(shmid, &hdr); + state = PGSharedMemoryAttach(shmid, UsedShmemSegAddr, &hdr); if (state != SHMSTATE_ATTACHED) elog(FATAL, "could not reattach to shared memory (key=%d, addr=%p): %m", (int) UsedShmemSegID, UsedShmemSegAddr);