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

Commit7de19fb

Browse files
committed
Use data directory inode number, not port, to select SysV resource keys.
This approach provides a much tighter binding between a data directoryand the associated SysV shared memory block (and SysV or named-POSIXsemaphores, if we're using those). Key collisions are still possible,but only between data directories stored on different filesystems,so the situation should be negligible in practice. More importantly,restarting the postmaster with a different port number no longerrisks failing to identify a relevant shared memory block, even whenpostmaster.pid has been removed. A standalone backend is likewisemuch more certain to detect conflicting leftover backends.(In the longer term, we might now think about deprecating the port asa cluster-wide value, so that one postmaster could support socketswith varying port numbers. But that's for another day.)The hazards fixed here apply only on Unix systems; our Windows codepaths already use identifiers derived from the data directory pathname rather than the port.src/test/recovery/t/017_shm.pl, which intends to test key-collisioncases, has been substantially rewritten since it can no longer usetwo postmasters with identical port numbers to trigger the case.Instead, use Perl's IPC::SharedMem module to create a conflictingshmem segment directly. The test script will be skipped if thatmodule is not available. (This means that some older buildfarmmembers won't run it, but I don't think that that results in anymeaningful coverage loss.)Patch by me; thanks to Noah Misch and Peter Eisentraut for discussionand review.Discussion:https://postgr.es/m/16908.1557521200@sss.pgh.pa.us
1 parent8b94dab commit7de19fb

File tree

12 files changed

+159
-124
lines changed

12 files changed

+159
-124
lines changed

‎src/backend/port/posix_sema.c

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include<semaphore.h>
3030
#include<signal.h>
3131
#include<unistd.h>
32+
#include<sys/stat.h>
3233

3334
#include"miscadmin.h"
3435
#include"storage/ipc.h"
@@ -181,10 +182,6 @@ PGSemaphoreShmemSize(int maxSemas)
181182
* are acquired here or in PGSemaphoreCreate, register an on_shmem_exit
182183
* callback to release them.
183184
*
184-
* The port number is passed for possible use as a key (for Posix, we use
185-
* it to generate the starting semaphore name). In a standalone backend,
186-
* zero will be passed.
187-
*
188185
* In the Posix implementation, we acquire semaphores on-demand; the
189186
* maxSemas parameter is just used to size the arrays. For unnamed
190187
* semaphores, there is an array of PGSemaphoreData structs in shared memory.
@@ -196,8 +193,22 @@ PGSemaphoreShmemSize(int maxSemas)
196193
* we don't have to expose the counters to other processes.)
197194
*/
198195
void
199-
PGReserveSemaphores(intmaxSemas,intport)
196+
PGReserveSemaphores(intmaxSemas)
200197
{
198+
structstatstatbuf;
199+
200+
/*
201+
* We use the data directory's inode number to seed the search for free
202+
* semaphore keys. This minimizes the odds of collision with other
203+
* postmasters, while maximizing the odds that we will detect and clean up
204+
* semaphores left over from a crashed postmaster in our own directory.
205+
*/
206+
if (stat(DataDir,&statbuf)<0)
207+
ereport(FATAL,
208+
(errcode_for_file_access(),
209+
errmsg("could not stat data directory \"%s\": %m",
210+
DataDir)));
211+
201212
#ifdefUSE_NAMED_POSIX_SEMAPHORES
202213
mySemPointers= (sem_t**)malloc(maxSemas*sizeof(sem_t*));
203214
if (mySemPointers==NULL)
@@ -215,7 +226,7 @@ PGReserveSemaphores(int maxSemas, int port)
215226

216227
numSems=0;
217228
maxSems=maxSemas;
218-
nextSemKey=port*1000;
229+
nextSemKey=statbuf.st_ino;
219230

220231
on_shmem_exit(ReleaseSemaphores,0);
221232
}

‎src/backend/port/sysv_sema.c

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include<signal.h>
1818
#include<unistd.h>
1919
#include<sys/file.h>
20+
#include<sys/stat.h>
2021
#ifdefHAVE_SYS_IPC_H
2122
#include<sys/ipc.h>
2223
#endif
@@ -301,10 +302,6 @@ PGSemaphoreShmemSize(int maxSemas)
301302
* are acquired here or in PGSemaphoreCreate, register an on_shmem_exit
302303
* callback to release them.
303304
*
304-
* The port number is passed for possible use as a key (for SysV, we use
305-
* it to generate the starting semaphore key). In a standalone backend,
306-
* zero will be passed.
307-
*
308305
* In the SysV implementation, we acquire semaphore sets on-demand; the
309306
* maxSemas parameter is just used to size the arrays. There is an array
310307
* of PGSemaphoreData structs in shared memory, and a postmaster-local array
@@ -314,8 +311,22 @@ PGSemaphoreShmemSize(int maxSemas)
314311
* have clobbered.)
315312
*/
316313
void
317-
PGReserveSemaphores(intmaxSemas,intport)
314+
PGReserveSemaphores(intmaxSemas)
318315
{
316+
structstatstatbuf;
317+
318+
/*
319+
* We use the data directory's inode number to seed the search for free
320+
* semaphore keys. This minimizes the odds of collision with other
321+
* postmasters, while maximizing the odds that we will detect and clean up
322+
* semaphores left over from a crashed postmaster in our own directory.
323+
*/
324+
if (stat(DataDir,&statbuf)<0)
325+
ereport(FATAL,
326+
(errcode_for_file_access(),
327+
errmsg("could not stat data directory \"%s\": %m",
328+
DataDir)));
329+
319330
/*
320331
* We must use ShmemAllocUnlocked(), since the spinlock protecting
321332
* ShmemAlloc() won't be ready yet. (This ordering is necessary when we
@@ -332,7 +343,7 @@ PGReserveSemaphores(int maxSemas, int port)
332343
if (mySemaSets==NULL)
333344
elog(PANIC,"out of memory");
334345
numSemaSets=0;
335-
nextSemaKey=port*1000;
346+
nextSemaKey=statbuf.st_ino;
336347
nextSemaNumber=SEMAS_PER_SET;/* force sema set alloc on 1st call */
337348

338349
on_shmem_exit(ReleaseSemaphores,0);

‎src/backend/port/sysv_shmem.c

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -390,11 +390,12 @@ PGSharedMemoryAttach(IpcMemoryId shmId,
390390

391391
/*
392392
* Try to attach to the segment and see if it matches our data directory.
393-
* This avoids key-conflict problems on machines that are running several
394-
* postmasters under the same userid and port number. (That would not
395-
* ordinarily happen in production, but it can happen during parallel
396-
* testing. Since our test setups don't open any TCP ports on Unix, such
397-
* cases don't conflict otherwise.)
393+
* This avoids any risk of duplicate-shmem-key conflicts on machines that
394+
* are running several postmasters under the same userid.
395+
*
396+
* (When we're called from PGSharedMemoryCreate, this stat call is
397+
* duplicative; but since this isn't a high-traffic case it's not worth
398+
* trying to optimize.)
398399
*/
399400
if (stat(DataDir,&statbuf)<0)
400401
returnSHMSTATE_ANALYSIS_FAILURE;/* can't stat; be conservative */
@@ -617,12 +618,9 @@ AnonymousShmemDetach(int status, Datum arg)
617618
* we do not fail upon collision with foreign shmem segments. The idea here
618619
* is to detect and re-use keys that may have been assigned by a crashed
619620
* postmaster or backend.
620-
*
621-
* The port number is passed for possible use as a key (for SysV, we use
622-
* it to generate the starting shmem key).
623621
*/
624622
PGShmemHeader*
625-
PGSharedMemoryCreate(Sizesize,intport,
623+
PGSharedMemoryCreate(Sizesize,
626624
PGShmemHeader**shim)
627625
{
628626
IpcMemoryKeyNextShmemSegID;
@@ -631,6 +629,17 @@ PGSharedMemoryCreate(Size size, int port,
631629
structstatstatbuf;
632630
Sizesysvsize;
633631

632+
/*
633+
* We use the data directory's ID info (inode and device numbers) to
634+
* positively identify shmem segments associated with this data dir, and
635+
* also as seeds for searching for a free shmem key.
636+
*/
637+
if (stat(DataDir,&statbuf)<0)
638+
ereport(FATAL,
639+
(errcode_for_file_access(),
640+
errmsg("could not stat data directory \"%s\": %m",
641+
DataDir)));
642+
634643
/* Complain if hugepages demanded but we can't possibly support them */
635644
#if !defined(MAP_HUGETLB)
636645
if (huge_pages==HUGE_PAGES_ON)
@@ -659,10 +668,10 @@ PGSharedMemoryCreate(Size size, int port,
659668
/*
660669
* Loop till we find a free IPC key. Trust CreateDataDirLockFile() to
661670
* ensure no more than one postmaster per data directory can enter this
662-
* loop simultaneously. (CreateDataDirLockFile() does notensure that,
663-
* but prefer fixing it over coping here.)
671+
* loop simultaneously. (CreateDataDirLockFile() does notentirely ensure
672+
*that,but prefer fixing it over coping here.)
664673
*/
665-
NextShmemSegID=1+port*1000;
674+
NextShmemSegID=statbuf.st_ino;
666675

667676
for (;;)
668677
{
@@ -748,11 +757,6 @@ PGSharedMemoryCreate(Size size, int port,
748757
hdr->dsm_control=0;
749758

750759
/* Fill in the data directory ID info, too */
751-
if (stat(DataDir,&statbuf)<0)
752-
ereport(FATAL,
753-
(errcode_for_file_access(),
754-
errmsg("could not stat data directory \"%s\": %m",
755-
DataDir)));
756760
hdr->device=statbuf.st_dev;
757761
hdr->inode=statbuf.st_ino;
758762

‎src/backend/port/win32_sema.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ PGSemaphoreShmemSize(int maxSemas)
4444
* process exits.
4545
*/
4646
void
47-
PGReserveSemaphores(intmaxSemas,intport)
47+
PGReserveSemaphores(intmaxSemas)
4848
{
4949
mySemSet= (HANDLE*)malloc(maxSemas*sizeof(HANDLE));
5050
if (mySemSet==NULL)

‎src/backend/port/win32_shmem.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ EnableLockPagesPrivilege(int elevel)
194194
* standard header.
195195
*/
196196
PGShmemHeader*
197-
PGSharedMemoryCreate(Sizesize,intport,
197+
PGSharedMemoryCreate(Sizesize,
198198
PGShmemHeader**shim)
199199
{
200200
void*memAddress;

‎src/backend/postmaster/postmaster.c

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ static void getInstallationPaths(const char *argv0);
385385
staticvoidcheckControlFile(void);
386386
staticPort*ConnCreate(intserverFd);
387387
staticvoidConnFree(Port*port);
388-
staticvoidreset_shared(intport);
388+
staticvoidreset_shared(void);
389389
staticvoidSIGHUP_handler(SIGNAL_ARGS);
390390
staticvoidpmdie(SIGNAL_ARGS);
391391
staticvoidreaper(SIGNAL_ARGS);
@@ -1175,7 +1175,7 @@ PostmasterMain(int argc, char *argv[])
11751175
/*
11761176
* Set up shared memory and semaphores.
11771177
*/
1178-
reset_shared(PostPortNumber);
1178+
reset_shared();
11791179

11801180
/*
11811181
* Estimate number of openable files. This must happen after setting up
@@ -2599,17 +2599,16 @@ InitProcessGlobals(void)
25992599
* reset_shared -- reset shared memory and semaphores
26002600
*/
26012601
staticvoid
2602-
reset_shared(intport)
2602+
reset_shared(void)
26032603
{
26042604
/*
26052605
* Create or re-create shared memory and semaphores.
26062606
*
26072607
* Note: in each "cycle of life" we will normally assign the same IPC keys
2608-
* (if using SysV shmem and/or semas), since the port number is used to
2609-
* determine IPC keys. This helps ensure that we will clean up dead IPC
2610-
* objects if the postmaster crashes and is restarted.
2608+
* (if using SysV shmem and/or semas). This helps ensure that we will
2609+
* clean up dead IPC objects if the postmaster crashes and is restarted.
26112610
*/
2612-
CreateSharedMemoryAndSemaphores(port);
2611+
CreateSharedMemoryAndSemaphores();
26132612
}
26142613

26152614

@@ -3934,7 +3933,7 @@ PostmasterStateMachine(void)
39343933
/* re-read control file into local memory */
39353934
LocalProcessControlFile(true);
39363935

3937-
reset_shared(PostPortNumber);
3936+
reset_shared();
39383937

39393938
StartupPID=StartupDataBase();
39403939
Assert(StartupPID!=0);
@@ -4962,7 +4961,7 @@ SubPostmasterMain(int argc, char *argv[])
49624961
InitProcess();
49634962

49644963
/* Attach process to shared data structures */
4965-
CreateSharedMemoryAndSemaphores(0);
4964+
CreateSharedMemoryAndSemaphores();
49664965

49674966
/* And run the backend */
49684967
BackendRun(&port);/* does not return */
@@ -4976,7 +4975,7 @@ SubPostmasterMain(int argc, char *argv[])
49764975
InitAuxiliaryProcess();
49774976

49784977
/* Attach process to shared data structures */
4979-
CreateSharedMemoryAndSemaphores(0);
4978+
CreateSharedMemoryAndSemaphores();
49804979

49814980
AuxiliaryProcessMain(argc-2,argv+2);/* does not return */
49824981
}
@@ -4989,7 +4988,7 @@ SubPostmasterMain(int argc, char *argv[])
49894988
InitProcess();
49904989

49914990
/* Attach process to shared data structures */
4992-
CreateSharedMemoryAndSemaphores(0);
4991+
CreateSharedMemoryAndSemaphores();
49934992

49944993
AutoVacLauncherMain(argc-2,argv+2);/* does not return */
49954994
}
@@ -5002,7 +5001,7 @@ SubPostmasterMain(int argc, char *argv[])
50025001
InitProcess();
50035002

50045003
/* Attach process to shared data structures */
5005-
CreateSharedMemoryAndSemaphores(0);
5004+
CreateSharedMemoryAndSemaphores();
50065005

50075006
AutoVacWorkerMain(argc-2,argv+2);/* does not return */
50085007
}
@@ -5020,7 +5019,7 @@ SubPostmasterMain(int argc, char *argv[])
50205019
InitProcess();
50215020

50225021
/* Attach process to shared data structures */
5023-
CreateSharedMemoryAndSemaphores(0);
5022+
CreateSharedMemoryAndSemaphores();
50245023

50255024
/* Fetch MyBgworkerEntry from shared memory */
50265025
shmem_slot=atoi(argv[1]+15);

‎src/backend/storage/ipc/ipci.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ RequestAddinShmemSpace(Size size)
9191
* This is a bit code-wasteful and could be cleaned up.)
9292
*/
9393
void
94-
CreateSharedMemoryAndSemaphores(intport)
94+
CreateSharedMemoryAndSemaphores(void)
9595
{
9696
PGShmemHeader*shim=NULL;
9797

@@ -163,14 +163,14 @@ CreateSharedMemoryAndSemaphores(int port)
163163
/*
164164
* Create the shmem segment
165165
*/
166-
seghdr=PGSharedMemoryCreate(size,port,&shim);
166+
seghdr=PGSharedMemoryCreate(size,&shim);
167167

168168
InitShmemAccess(seghdr);
169169

170170
/*
171171
* Create semaphores
172172
*/
173-
PGReserveSemaphores(numSemas,port);
173+
PGReserveSemaphores(numSemas);
174174

175175
/*
176176
* If spinlocks are disabled, initialize emulation layer (which

‎src/backend/utils/init/postinit.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -445,12 +445,10 @@ InitCommunication(void)
445445
if (!IsUnderPostmaster)/* postmaster already did this */
446446
{
447447
/*
448-
* We're running a postgres bootstrap process or a standalone backend.
449-
* Though we won't listen on PostPortNumber, use it to select a shmem
450-
* key. This increases the chance of detecting a leftover live
451-
* backend of this DataDir.
448+
* We're running a postgres bootstrap process or a standalone backend,
449+
* so we need to set up shmem.
452450
*/
453-
CreateSharedMemoryAndSemaphores(PostPortNumber);
451+
CreateSharedMemoryAndSemaphores();
454452
}
455453
}
456454

‎src/include/storage/ipc.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,6 @@ extern void on_exit_reset(void);
7676
/* ipci.c */
7777
externPGDLLIMPORTshmem_startup_hook_typeshmem_startup_hook;
7878

79-
externvoidCreateSharedMemoryAndSemaphores(intport);
79+
externvoidCreateSharedMemoryAndSemaphores(void);
8080

8181
#endif/* IPC_H */

‎src/include/storage/pg_sema.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ typedef HANDLE PGSemaphore;
4141
externSizePGSemaphoreShmemSize(intmaxSemas);
4242

4343
/* Module initialization (called during postmaster start or shmem reinit) */
44-
externvoidPGReserveSemaphores(intmaxSemas,intport);
44+
externvoidPGReserveSemaphores(intmaxSemas);
4545

4646
/* Allocate a PGSemaphore structure with initial count 1 */
4747
externPGSemaphorePGSemaphoreCreate(void);

‎src/include/storage/pg_shmem.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ extern void PGSharedMemoryReAttach(void);
8282
externvoidPGSharedMemoryNoReAttach(void);
8383
#endif
8484

85-
externPGShmemHeader*PGSharedMemoryCreate(Sizesize,intport,
85+
externPGShmemHeader*PGSharedMemoryCreate(Sizesize,
8686
PGShmemHeader**shim);
8787
externboolPGSharedMemoryIsInUse(unsigned longid1,unsigned longid2);
8888
externvoidPGSharedMemoryDetach(void);

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp