Skip to content

Commit be7b284

Browse files
tglsfdctglsfdc
committed
Make the different Unix-y semaphore implementations ABI-compatible.
Previously, the "sem" field of PGPROC varied in size depending on which kernel semaphore API we were using. That was okay as long as there was only one likely choice per platform, but in the wake of commit ecb0d20, that assumption seems rather shaky. It doesn't seem out of the question anymore that an extension compiled against one API choice might be loaded into a postmaster built with another choice. Moreover, this prevents any possibility of selecting the semaphore API at postmaster startup, which might be something we want to do in future. Hence, change PGPROC.sem to be PGSemaphore (i.e. a pointer) for all Unix semaphore APIs, and turn the pointed-to data into an opaque struct whose contents are only known within the responsible modules. For the SysV and unnamed-POSIX APIs, the pointed-to data has to be allocated elsewhere in shared memory, which takes a little bit of rejiggering of the InitShmemAllocation code sequence. (I invented a ShmemAllocUnlocked() function to make that a little cleaner than it used to be. That function is not meant for any uses other than the ones it has now, but it beats having InitShmemAllocation() know explicitly about allocation of space for semaphores and spinlocks.) This change means an extra indirection to access the semaphore data, but since we only touch that when blocking or awakening a process, there shouldn't be any meaningful performance penalty. Moreover, at least for the unnamed-POSIX case on Linux, the sem_t type is quite a bit wider than a pointer, so this reduces sizeof(PGPROC) which seems like a good thing. For the named-POSIX API, there's effectively no change: the PGPROC.sem field was and still is a pointer to something returned by sem_open() in the postmaster's memory space. Document and check the pre-existing limitation that this case can't work in EXEC_BACKEND mode. It did not seem worth unifying the Windows semaphore ABI with the Unix cases, since there's no likelihood of needing ABI compatibility much less runtime switching across those cases. However, we can simplify the Windows code a bit if we define PGSemaphore as being directly a HANDLE, rather than pointer to HANDLE, so let's do that while we're here. (This also ends up being no change in what's physically stored in PGPROC.sem. We're just moving the HANDLE fetch from callees to callers.) It would take a bunch of additional code shuffling to get to the point of actually choosing a semaphore API at postmaster start, but the effects of that would now be localized in the port/XXX_sema.c files, so it seems like fit material for a separate patch. The need for it is unproven as yet, anyhow, whereas the ABI risk to extensions seems real enough. Discussion: https://postgr.es/m/4029.1481413370@sss.pgh.pa.us
1 parent 06e1848 commit be7b284

File tree

14 files changed

+268
-123
lines changed

14 files changed

+268
-123
lines changed

‎src/backend/port/posix_sema.c

Lines changed: 81 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@
66
* We prefer the unnamed style of POSIX semaphore (the kind made with
77
* sem_init). We can cope with the kind made with sem_open, however.
88
*
9+
* In either implementation, typedef PGSemaphore is equivalent to "sem_t *".
10+
* With unnamed semaphores, the sem_t structs live in an array in shared
11+
* memory. With named semaphores, that's not true because we cannot persuade
12+
* sem_open to do its allocation there. Therefore, the named-semaphore code
13+
* *does not cope with EXEC_BACKEND*. The sem_t structs will just be in the
14+
* postmaster's private memory, where they are successfully inherited by
15+
* forked backends, but they could not be accessed by exec'd backends.
16+
*
917
*
1018
* Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group
1119
* Portions Copyright (c) 1994, Regents of the University of California
@@ -18,28 +26,38 @@
1826
#include "postgres.h"
1927

2028
#include <fcntl.h>
29+
#include <semaphore.h>
2130
#include <signal.h>
2231
#include <unistd.h>
2332

2433
#include "miscadmin.h"
2534
#include "storage/ipc.h"
2635
#include "storage/pg_sema.h"
36+
#include "storage/shmem.h"
2737

2838

29-
#ifdef USE_NAMED_POSIX_SEMAPHORES
30-
/* PGSemaphore is pointer to pointer to sem_t */
31-
#define PG_SEM_REF(x) (*(x))
32-
#else
33-
/* PGSemaphore is pointer to sem_t */
34-
#define PG_SEM_REF(x) (x)
39+
/* see file header comment */
40+
#if defined(USE_NAMED_POSIX_SEMAPHORES) && defined(EXEC_BACKEND)
41+
#error cannot use named POSIX semaphores with EXEC_BACKEND
3542
#endif
3643

44+
/* typedef PGSemaphore is equivalent to pointer to sem_t */
45+
typedef struct PGSemaphoreData
46+
{
47+
sem_t pgsem;
48+
} PGSemaphoreData;
49+
50+
#define PG_SEM_REF(x) (&(x)->pgsem)
3751

3852
#define IPCProtection (0600) /* access/modify by user only */
3953

54+
#ifdef USE_NAMED_POSIX_SEMAPHORES
4055
static sem_t **mySemPointers; /* keep track of created semaphores */
56+
#else
57+
static PGSemaphore sharedSemas; /* array of PGSemaphoreData in shared memory */
58+
#endif
4159
static int numSems; /* number of semas acquired so far */
42-
static int maxSems; /* allocated size of mySemaPointers array */
60+
static int maxSems; /* allocated size of above arrays */
4361
static int nextSemKey; /* next name to try */
4462

4563

@@ -133,6 +151,21 @@ PosixSemaphoreKill(sem_t * sem)
133151
}
134152

135153

154+
/*
155+
* Report amount of shared memory needed for semaphores
156+
*/
157+
Size
158+
PGSemaphoreShmemSize(int maxSemas)
159+
{
160+
#ifdef USE_NAMED_POSIX_SEMAPHORES
161+
/* No shared memory needed in this case */
162+
return 0;
163+
#else
164+
/* Need a PGSemaphoreData per semaphore */
165+
return mul_size(maxSemas, sizeof(PGSemaphoreData));
166+
#endif
167+
}
168+
136169
/*
137170
* PGReserveSemaphores --- initialize semaphore support
138171
*
@@ -147,15 +180,33 @@ PosixSemaphoreKill(sem_t * sem)
147180
* zero will be passed.
148181
*
149182
* In the Posix implementation, we acquire semaphores on-demand; the
150-
* maxSemas parameter is just used to size the array that keeps track of
151-
* acquired semas for subsequent releasing.
183+
* maxSemas parameter is just used to size the arrays. For unnamed
184+
* semaphores, there is an array of PGSemaphoreData structs in shared memory.
185+
* For named semaphores, we keep a postmaster-local array of sem_t pointers,
186+
* which we use for releasing the semphores when done.
187+
* (This design minimizes the dependency of postmaster shutdown on the
188+
* contents of shared memory, which a failed backend might have clobbered.
189+
* We can't do much about the possibility of sem_destroy() crashing, but
190+
* we don't have to expose the counters to other processes.)
152191
*/
153192
void
154193
PGReserveSemaphores(int maxSemas, int port)
155194
{
195+
#ifdef USE_NAMED_POSIX_SEMAPHORES
156196
mySemPointers = (sem_t **) malloc(maxSemas * sizeof(sem_t *));
157197
if (mySemPointers == NULL)
158198
elog(PANIC, "out of memory");
199+
#else
200+
201+
/*
202+
* We must use ShmemAllocUnlocked(), since the spinlock protecting
203+
* ShmemAlloc() won't be ready yet. (This ordering is necessary when we
204+
* are emulating spinlocks with semaphores.)
205+
*/
206+
sharedSemas = (PGSemaphore)
207+
ShmemAllocUnlocked(PGSemaphoreShmemSize(maxSemas));
208+
#endif
209+
159210
numSems = 0;
160211
maxSems = maxSemas;
161212
nextSemKey = port * 1000;
@@ -173,19 +224,27 @@ ReleaseSemaphores(int status, Datum arg)
173224
{
174225
int i;
175226

227+
#ifdef USE_NAMED_POSIX_SEMAPHORES
176228
for (i = 0; i < numSems; i++)
177229
PosixSemaphoreKill(mySemPointers[i]);
178230
free(mySemPointers);
231+
#endif
232+
233+
#ifdef USE_UNNAMED_POSIX_SEMAPHORES
234+
for (i = 0; i < numSems; i++)
235+
PosixSemaphoreKill(PG_SEM_REF(sharedSemas + i));
236+
#endif
179237
}
180238

181239
/*
182240
* PGSemaphoreCreate
183241
*
184-
* Initialize a PGSemaphore structure to represent a sema with count 1
242+
* Allocate a PGSemaphore structure with initial count 1
185243
*/
186-
void
187-
PGSemaphoreCreate(PGSemaphore sema)
244+
PGSemaphore
245+
PGSemaphoreCreate(void)
188246
{
247+
PGSemaphore sema;
189248
sem_t *newsem;
190249

191250
/* Can't do this in a backend, because static state is postmaster's */
@@ -195,14 +254,19 @@ PGSemaphoreCreate(PGSemaphore sema)
195254
elog(PANIC, "too many semaphores created");
196255

197256
#ifdef USE_NAMED_POSIX_SEMAPHORES
198-
*sema = newsem = PosixSemaphoreCreate();
257+
newsem = PosixSemaphoreCreate();
258+
/* Remember new sema for ReleaseSemaphores */
259+
mySemPointers[numSems] = newsem;
260+
sema = (PGSemaphore) newsem;
199261
#else
200-
PosixSemaphoreCreate(sema);
201-
newsem = sema;
262+
sema = &sharedSemas[numSems];
263+
newsem = PG_SEM_REF(sema);
264+
PosixSemaphoreCreate(newsem);
202265
#endif
203266

204-
/* Remember new sema for ReleaseSemaphores */
205-
mySemPointers[numSems++] = newsem;
267+
numSems++;
268+
269+
return sema;
206270
}
207271

208272
/*

‎src/backend/port/sysv_sema.c

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,15 @@
2727
#include "miscadmin.h"
2828
#include "storage/ipc.h"
2929
#include "storage/pg_sema.h"
30+
#include "storage/shmem.h"
3031

3132

33+
typedef struct PGSemaphoreData
34+
{
35+
int semId; /* semaphore set identifier */
36+
int semNum; /* semaphore number within set */
37+
} PGSemaphoreData;
38+
3239
#ifndef HAVE_UNION_SEMUN
3340
union semun
3441
{
@@ -54,6 +61,9 @@ typedef int IpcSemaphoreId; /* semaphore ID returned by semget(2) */
5461
#define PGSemaMagic 537 /* must be less than SEMVMX */
5562

5663

64+
static PGSemaphore sharedSemas; /* array of PGSemaphoreData in shared memory */
65+
static int numSharedSemas; /* number of PGSemaphoreDatas used so far */
66+
static int maxSharedSemas; /* allocated size of PGSemaphoreData array */
5767
static IpcSemaphoreId *mySemaSets; /* IDs of sema sets acquired so far */
5868
static int numSemaSets; /* number of sema sets acquired so far */
5969
static int maxSemaSets; /* allocated size of mySemaSets array */
@@ -273,6 +283,15 @@ IpcSemaphoreCreate(int numSems)
273283
}
274284

275285

286+
/*
287+
* Report amount of shared memory needed for semaphores
288+
*/
289+
Size
290+
PGSemaphoreShmemSize(int maxSemas)
291+
{
292+
return mul_size(maxSemas, sizeof(PGSemaphoreData));
293+
}
294+
276295
/*
277296
* PGReserveSemaphores --- initialize semaphore support
278297
*
@@ -287,12 +306,26 @@ IpcSemaphoreCreate(int numSems)
287306
* zero will be passed.
288307
*
289308
* In the SysV implementation, we acquire semaphore sets on-demand; the
290-
* maxSemas parameter is just used to size the array that keeps track of
291-
* acquired sets for subsequent releasing.
309+
* maxSemas parameter is just used to size the arrays. There is an array
310+
* of PGSemaphoreData structs in shared memory, and a postmaster-local array
311+
* with one entry per SysV semaphore set, which we use for releasing the
312+
* semaphore sets when done. (This design ensures that postmaster shutdown
313+
* doesn't rely on the contents of shared memory, which a failed backend might
314+
* have clobbered.)
292315
*/
293316
void
294317
PGReserveSemaphores(int maxSemas, int port)
295318
{
319+
/*
320+
* We must use ShmemAllocUnlocked(), since the spinlock protecting
321+
* ShmemAlloc() won't be ready yet. (This ordering is necessary when we
322+
* are emulating spinlocks with semaphores.)
323+
*/
324+
sharedSemas = (PGSemaphore)
325+
ShmemAllocUnlocked(PGSemaphoreShmemSize(maxSemas));
326+
numSharedSemas = 0;
327+
maxSharedSemas = maxSemas;
328+
296329
maxSemaSets = (maxSemas + SEMAS_PER_SET - 1) / SEMAS_PER_SET;
297330
mySemaSets = (IpcSemaphoreId *)
298331
malloc(maxSemaSets * sizeof(IpcSemaphoreId));
@@ -323,11 +356,13 @@ ReleaseSemaphores(int status, Datum arg)
323356
/*
324357
* PGSemaphoreCreate
325358
*
326-
* Initialize a PGSemaphore structure to represent a sema with count 1
359+
* Allocate a PGSemaphore structure with initial count 1
327360
*/
328-
void
329-
PGSemaphoreCreate(PGSemaphore sema)
361+
PGSemaphore
362+
PGSemaphoreCreate(void)
330363
{
364+
PGSemaphore sema;
365+
331366
/* Can't do this in a backend, because static state is postmaster's */
332367
Assert(!IsUnderPostmaster);
333368

@@ -340,11 +375,17 @@ PGSemaphoreCreate(PGSemaphore sema)
340375
numSemaSets++;
341376
nextSemaNumber = 0;
342377
}
378+
/* Use the next shared PGSemaphoreData */
379+
if (numSharedSemas >= maxSharedSemas)
380+
elog(PANIC, "too many semaphores created");
381+
sema = &sharedSemas[numSharedSemas++];
343382
/* Assign the next free semaphore in the current set */
344383
sema->semId = mySemaSets[numSemaSets - 1];
345384
sema->semNum = nextSemaNumber++;
346385
/* Initialize it to count 1 */
347386
IpcSemaphoreInitialize(sema->semId, sema->semNum, 1);
387+
388+
return sema;
348389
}
349390

350391
/*

‎src/backend/port/win32_sema.c

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,17 @@ static int maxSems; /* allocated size of mySemaSet array */
2323

2424
static void ReleaseSemaphores(int code, Datum arg);
2525

26+
27+
/*
28+
* Report amount of shared memory needed for semaphores
29+
*/
30+
Size
31+
PGSemaphoreShmemSize(int maxSemas)
32+
{
33+
/* No shared memory needed on Windows */
34+
return 0;
35+
}
36+
2637
/*
2738
* PGReserveSemaphores --- initialize semaphore support
2839
*
@@ -62,10 +73,10 @@ ReleaseSemaphores(int code, Datum arg)
6273
/*
6374
* PGSemaphoreCreate
6475
*
65-
* Initialize a PGSemaphore structure to represent a sema with count 1
76+
* Allocate a PGSemaphore structure with initial count 1
6677
*/
67-
void
68-
PGSemaphoreCreate(PGSemaphore sema)
78+
PGSemaphore
79+
PGSemaphoreCreate(void)
6980
{
7081
HANDLE cur_handle;
7182
SECURITY_ATTRIBUTES sec_attrs;
@@ -86,12 +97,14 @@ PGSemaphoreCreate(PGSemaphore sema)
8697
if (cur_handle)
8798
{
8899
/* Successfully done */
89-
*sema = cur_handle;
90100
mySemSet[numSems++] = cur_handle;
91101
}
92102
else
93103
ereport(PANIC,
94-
(errmsg("could not create semaphore: error code %lu", GetLastError())));
104+
(errmsg("could not create semaphore: error code %lu",
105+
GetLastError())));
106+
107+
return (PGSemaphore) cur_handle;
95108
}
96109

97110
/*
@@ -106,7 +119,8 @@ PGSemaphoreReset(PGSemaphore sema)
106119
* There's no direct API for this in Win32, so we have to ratchet the
107120
* semaphore down to 0 with repeated trylock's.
108121
*/
109-
while (PGSemaphoreTryLock(sema));
122+
while (PGSemaphoreTryLock(sema))
123+
/* loop */ ;
110124
}
111125

112126
/*
@@ -127,7 +141,7 @@ PGSemaphoreLock(PGSemaphore sema)
127141
* pending signals are serviced.
128142
*/
129143
wh[0] = pgwin32_signal_event;
130-
wh[1] = *sema;
144+
wh[1] = sema;
131145

132146
/*
133147
* As in other implementations of PGSemaphoreLock, we need to check for
@@ -182,9 +196,10 @@ PGSemaphoreLock(PGSemaphore sema)
182196
void
183197
PGSemaphoreUnlock(PGSemaphore sema)
184198
{
185-
if (!ReleaseSemaphore(*sema, 1, NULL))
199+
if (!ReleaseSemaphore(sema, 1, NULL))
186200
ereport(FATAL,
187-
(errmsg("could not unlock semaphore: error code %lu", GetLastError())));
201+
(errmsg("could not unlock semaphore: error code %lu",
202+
GetLastError())));
188203
}
189204

190205
/*
@@ -197,7 +212,7 @@ PGSemaphoreTryLock(PGSemaphore sema)
197212
{
198213
DWORD ret;
199214

200-
ret = WaitForSingleObject(*sema, 0);
215+
ret = WaitForSingleObject(sema, 0);
201216

202217
if (ret == WAIT_OBJECT_0)
203218
{
@@ -213,7 +228,8 @@ PGSemaphoreTryLock(PGSemaphore sema)
213228

214229
/* Otherwise we are in trouble */
215230
ereport(FATAL,
216-
(errmsg("could not try-lock semaphore: error code %lu", GetLastError())));
231+
(errmsg("could not try-lock semaphore: error code %lu",
232+
GetLastError())));
217233

218234
/* keep compiler quiet */
219235
return false;

‎src/backend/postmaster/postmaster.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ typedef struct
484484
VariableCache ShmemVariableCache;
485485
Backend *ShmemBackendArray;
486486
#ifndef HAVE_SPINLOCKS
487-
PGSemaphore SpinlockSemaArray;
487+
PGSemaphore *SpinlockSemaArray;
488488
#endif
489489
int NamedLWLockTrancheRequests;
490490
NamedLWLockTranche *NamedLWLockTrancheArray;

0 commit comments

Comments
 (0)