database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-03 20:02:46 +03:00
Code Activity

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

Browse Source 
This reverts commits 2f932f71d9,
16ee6eaf80 and
6f0e190056.  The buildfarm has revealed
several bugs.  Back-patch like the original commits.

Discussion: https://postgr.es/m/20190404145319.GA1720877@rfd.leadboat.com
This commit is contained in:
Noah Misch
2019-04-05 00:00:52 -07:00
parent 1f63f69c43
commit 9ec582b64d
7 changed files with 140 additions and 168 deletions
Download Patch File Download Diff File Expand all files Collapse all files

261
src/backend/port/sysv_shmem.c
View File

@ -70,26 +70,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;
@ -102,8 +82,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);
/*
@ -307,36 +287,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.
@ -349,7 +304,7 @@ PGSharedMemoryAttach(IpcMemoryId shmId,
* exists.
*/
if (errno == EINVAL)
return SHMSTATE_ENOENT;
return false;
/*
* EACCES implies that the segment belongs to some other userid, which
@ -357,7 +312,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)
@ -368,7 +323,7 @@ PGSharedMemoryAttach(IpcMemoryId shmId,
*/
#ifdef HAVE_LINUX_EIDRM_BUG
if (errno == EIDRM)
return SHMSTATE_ENOENT;
return false;
#endif
/*
@ -376,26 +331,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 ||
@ -403,12 +357,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
@ -584,21 +542,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;
@ -629,20 +591,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)
@ -650,71 +603,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;
@ -774,8 +714,7 @@ void
PGSharedMemoryReAttach(void)
{
IpcMemoryId shmid;
PGShmemHeader *hdr;
IpcMemoryState state;
void *hdr;
void *origUsedShmemSegAddr = UsedShmemSegAddr;
Assert(UsedShmemSegAddr != NULL);
@ -788,18 +727,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 */
}
@ -875,3 +810,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;
}

7
src/backend/port/win32_shmem.c
View File

@ -109,9 +109,14 @@ PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2)
*
* 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;

12
src/backend/postmaster/postmaster.c
View File

@ -2517,7 +2517,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);
}
@ -4804,7 +4804,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 */
@ -4818,7 +4818,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 */
}
@ -4831,7 +4831,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 */
}
@ -4844,7 +4844,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 */
}
@ -4862,7 +4862,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);

14
src/backend/storage/ipc/ipci.c
View File

@ -85,9 +85,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;
@ -152,7 +155,7 @@ CreateSharedMemoryAndSemaphores(int port)
/*
* Create the shmem segment
*/
seghdr = PGSharedMemoryCreate(size, port, &shim);
seghdr = PGSharedMemoryCreate(size, makePrivate, port, &shim);
InitShmemAccess(seghdr);
@ -167,9 +170,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
}

6
src/backend/utils/init/postinit.c
View File

@ -402,11 +402,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);
}
}

2
src/include/storage/ipc.h
View File

@ -75,6 +75,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 */

6
src/include/storage/pg_shmem.h
View File

@ -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);