Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit803f90a

Browse files
committed
Cope with EINVAL and EIDRM shmat() failures in PGSharedMemoryAttach.
There's a very old race condition in our code to see whether a pre-existingshared memory segment is still in use by a conflicting postmaster: it'spossible for the other postmaster to remove the segment in between ourshmctl() and shmat() calls. It's a narrow window, and there's no riskunless both postmasters are using the same port number, but that's possibleduring parallelized "make check" tests. (Note that while the TAP teststake some pains to choose a randomized port number, pg_regress doesn't.)If it does happen, we treated that as an unexpected case and errored out.To fix, allow EINVAL to be treated as segment-not-present, and the samefor EIDRM on Linux. AFAICS, the considerations here are basicallyidentical to the checks for acceptable shmctl() failures, so I documentedand coded it that way.While at it, adjust PGSharedMemoryAttach's API to remove its undocumenteddependency on UsedShmemSegAddr in favor of passing the attach addressexplicitly. This makes it easier to be sure we're using a null shmaddrwhen probing for segment conflicts (thus avoiding questions about whatEINVAL means). I don't think there was a bug there, but it requiredfragile assumptions about the state of UsedShmemSegAddr duringPGSharedMemoryIsInUse.Commitc098509 may have made this failure more probable by applyingthe conflicting-segment tests more often. Hence, back-patch to allsupported branches, as that was.Discussion:https://postgr.es/m/22224.1557340366@sss.pgh.pa.us
1 parente7eed0b commit803f90a

File tree

1 file changed

+47
-23
lines changed

1 file changed

+47
-23
lines changed

‎src/backend/port/sysv_shmem.c

Lines changed: 47 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ static void *InternalIpcMemoryCreate(IpcMemoryKey memKey, Size size);
104104
staticvoidIpcMemoryDetach(intstatus,Datumshmaddr);
105105
staticvoidIpcMemoryDelete(intstatus,DatumshmId);
106106
staticIpcMemoryStatePGSharedMemoryAttach(IpcMemoryIdshmId,
107+
void*attachAt,
107108
PGShmemHeader**addr);
108109

109110

@@ -311,7 +312,7 @@ PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2)
311312
PGShmemHeader*memAddress;
312313
IpcMemoryStatestate;
313314

314-
state=PGSharedMemoryAttach((IpcMemoryId)id2,&memAddress);
315+
state=PGSharedMemoryAttach((IpcMemoryId)id2,NULL,&memAddress);
315316
if (memAddress&&shmdt(memAddress)<0)
316317
elog(LOG,"shmdt(%p) failed: %m",memAddress);
317318
switch (state)
@@ -327,9 +328,17 @@ PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2)
327328
return true;
328329
}
329330

330-
/* See comment at IpcMemoryState. */
331+
/*
332+
* Test for a segment with id shmId; see comment at IpcMemoryState.
333+
*
334+
* If the segment exists, we'll attempt to attach to it, using attachAt
335+
* if that's not NULL (but it's best to pass NULL if possible).
336+
*
337+
* *addr is set to the segment memory address if we attached to it, else NULL.
338+
*/
331339
staticIpcMemoryState
332340
PGSharedMemoryAttach(IpcMemoryIdshmId,
341+
void*attachAt,
333342
PGShmemHeader**addr)
334343
{
335344
structshmid_dsshmStat;
@@ -339,8 +348,7 @@ PGSharedMemoryAttach(IpcMemoryId shmId,
339348
*addr=NULL;
340349

341350
/*
342-
* We detect whether a shared memory segment is in use by seeing whether
343-
* it (a) exists and (b) has any processes attached to it.
351+
* First, try to stat the shm segment ID, to see if it exists at all.
344352
*/
345353
if (shmctl(shmId,IPC_STAT,&shmStat)<0)
346354
{
@@ -373,34 +381,48 @@ PGSharedMemoryAttach(IpcMemoryId shmId,
373381
#endif
374382

375383
/*
376-
* Otherwise, we had better assume that the segment is in use. The
377-
* only likely case is EIDRM, which implies that the segment has been
378-
* IPC_RMID'd but there are still processes attached to it.
384+
* Otherwise, we had better assume that the segment is in use. The
385+
* only likely case is (non-Linux, assumed spec-compliant) EIDRM,
386+
* which implies that the segment has been IPC_RMID'd but there are
387+
* still processes attached to it.
379388
*/
380389
returnSHMSTATE_ANALYSIS_FAILURE;
381390
}
382391

383392
/*
384393
* Try to attach to the segment and see if it matches our data directory.
385394
* This avoids shmid-conflict problems on machines that are running
386-
* several postmasters under the same userid.
395+
* several postmasters under the same userid and port number. (That would
396+
* not ordinarily happen in production, but it can happen during parallel
397+
* testing. Since our test setups don't open any TCP ports on Unix, such
398+
* cases don't conflict otherwise.)
387399
*/
388400
if (stat(DataDir,&statbuf)<0)
389401
returnSHMSTATE_ANALYSIS_FAILURE;/* can't stat; be conservative */
390402

391-
/*
392-
* Attachment fails if we have no write permission. Since that will never
393-
* happen with Postgres IPCProtection, such a failure shows the segment is
394-
* not a Postgres segment. If attachment fails for some other reason, be
395-
* conservative.
396-
*/
397-
hdr= (PGShmemHeader*)shmat(shmId,UsedShmemSegAddr,PG_SHMAT_FLAGS);
403+
hdr= (PGShmemHeader*)shmat(shmId,attachAt,PG_SHMAT_FLAGS);
398404
if (hdr== (PGShmemHeader*)-1)
399405
{
406+
/*
407+
* Attachment failed. The cases we're interested in are the same as
408+
* for the shmctl() call above. In particular, note that the owning
409+
* postmaster could have terminated and removed the segment between
410+
* shmctl() and shmat().
411+
*
412+
* If attachAt isn't NULL, it's possible that EINVAL reflects a
413+
* problem with that address not a vanished segment, so it's best to
414+
* pass NULL when probing for conflicting segments.
415+
*/
416+
if (errno==EINVAL)
417+
returnSHMSTATE_ENOENT;/* segment disappeared */
400418
if (errno==EACCES)
401-
returnSHMSTATE_FOREIGN;
402-
else
403-
returnSHMSTATE_ANALYSIS_FAILURE;
419+
returnSHMSTATE_FOREIGN;/* must be non-Postgres */
420+
#ifdefHAVE_LINUX_EIDRM_BUG
421+
if (errno==EIDRM)
422+
returnSHMSTATE_ENOENT;/* segment disappeared */
423+
#endif
424+
/* Otherwise, be conservative. */
425+
returnSHMSTATE_ANALYSIS_FAILURE;
404426
}
405427
*addr=hdr;
406428

@@ -415,6 +437,11 @@ PGSharedMemoryAttach(IpcMemoryId shmId,
415437
returnSHMSTATE_FOREIGN;
416438
}
417439

440+
/*
441+
* It does match our data directory, so now test whether any processes are
442+
* still attached to it. (We are, now, but the shm_nattch result is from
443+
* before we attached to it.)
444+
*/
418445
returnshmStat.shm_nattch==0 ?SHMSTATE_UNATTACHED :SHMSTATE_ATTACHED;
419446
}
420447

@@ -633,9 +660,6 @@ PGSharedMemoryCreate(Size size, int port,
633660
sysvsize=size;
634661
#endif
635662

636-
/* Make sure PGSharedMemoryAttach doesn't fail without need */
637-
UsedShmemSegAddr=NULL;
638-
639663
/*
640664
* Loop till we find a free IPC key. Trust CreateDataDirLockFile() to
641665
* ensure no more than one postmaster per data directory can enter this
@@ -669,7 +693,7 @@ PGSharedMemoryCreate(Size size, int port,
669693
state=SHMSTATE_FOREIGN;
670694
}
671695
else
672-
state=PGSharedMemoryAttach(shmid,&oldhdr);
696+
state=PGSharedMemoryAttach(shmid,NULL,&oldhdr);
673697

674698
switch (state)
675699
{
@@ -799,7 +823,7 @@ PGSharedMemoryReAttach(void)
799823
if (shmid<0)
800824
state=SHMSTATE_FOREIGN;
801825
else
802-
state=PGSharedMemoryAttach(shmid,&hdr);
826+
state=PGSharedMemoryAttach(shmid,UsedShmemSegAddr,&hdr);
803827
if (state!=SHMSTATE_ATTACHED)
804828
elog(FATAL,"could not reattach to shared memory (key=%d, addr=%p): %m",
805829
(int)UsedShmemSegID,UsedShmemSegAddr);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp