diff options
| -rw-r--r-- | src/backend/port/posix_sema.c | 12 | ||||
| -rw-r--r-- | src/backend/port/sysv_sema.c | 43 | ||||
| -rw-r--r-- | src/backend/port/win32_sema.c | 6 | ||||
| -rw-r--r-- | src/backend/storage/lmgr/lwlock.c | 12 | ||||
| -rw-r--r-- | src/include/storage/pg_sema.h | 2 |
5 files changed, 12 insertions, 63 deletions
diff --git a/src/backend/port/posix_sema.c b/src/backend/port/posix_sema.c index 633bf5afebb..ad9c80a8b5b 100644 --- a/src/backend/port/posix_sema.c +++ b/src/backend/port/posix_sema.c @@ -236,22 +236,14 @@ PGSemaphoreReset(PGSemaphore sema) * Lock a semaphore (decrement count), blocking if count would be < 0 */ void -PGSemaphoreLock(PGSemaphore sema, bool interruptOK) +PGSemaphoreLock(PGSemaphore sema) { int errStatus; - /* - * See notes in sysv_sema.c's implementation of PGSemaphoreLock. Just as - * that code does for semop(), we handle both the case where sem_wait() - * returns errno == EINTR after a signal, and the case where it just keeps - * waiting. - */ + /* See notes in sysv_sema.c's implementation of PGSemaphoreLock. */ do { - ImmediateInterruptOK = interruptOK; - CHECK_FOR_INTERRUPTS(); errStatus = sem_wait(PG_SEM_REF(sema)); - ImmediateInterruptOK = false; } while (errStatus < 0 && errno == EINTR); if (errStatus < 0) diff --git a/src/backend/port/sysv_sema.c b/src/backend/port/sysv_sema.c index 5a0193f06b1..ba46c640fc9 100644 --- a/src/backend/port/sysv_sema.c +++ b/src/backend/port/sysv_sema.c @@ -364,7 +364,7 @@ PGSemaphoreReset(PGSemaphore sema) * Lock a semaphore (decrement count), blocking if count would be < 0 */ void -PGSemaphoreLock(PGSemaphore sema, bool interruptOK) +PGSemaphoreLock(PGSemaphore sema) { int errStatus; struct sembuf sops; @@ -378,48 +378,13 @@ PGSemaphoreLock(PGSemaphore sema, bool interruptOK) * from the operation prematurely because we were sent a signal. So we * try and lock the semaphore again. * - * Each time around the loop, we check for a cancel/die interrupt. On - * some platforms, if such an interrupt comes in while we are waiting, it - * will cause the semop() call to exit with errno == EINTR, allowing us to - * service the interrupt (if not in a critical section already) during the - * next loop iteration. - * - * Once we acquire the lock, we do NOT check for an interrupt before - * returning. The caller needs to be able to record ownership of the lock - * before any interrupt can be accepted. - * - * There is a window of a few instructions between CHECK_FOR_INTERRUPTS - * and entering the semop() call. If a cancel/die interrupt occurs in - * that window, we would fail to notice it until after we acquire the lock - * (or get another interrupt to escape the semop()). We can avoid this - * problem by temporarily setting ImmediateInterruptOK to true before we - * do CHECK_FOR_INTERRUPTS; then, a die() interrupt in this interval will - * execute directly. However, there is a huge pitfall: there is another - * window of a few instructions after the semop() before we are able to - * reset ImmediateInterruptOK. If an interrupt occurs then, we'll lose - * control, which means that the lock has been acquired but our caller did - * not get a chance to record the fact. Therefore, we only set - * ImmediateInterruptOK if the caller tells us it's OK to do so, ie, the - * caller does not need to record acquiring the lock. (This is currently - * true for lockmanager locks, since the process that granted us the lock - * did all the necessary state updates. It's not true for SysV semaphores - * used to implement LW locks or emulate spinlocks --- but the wait time - * for such locks should not be very long, anyway.) - * - * On some platforms, signals marked SA_RESTART (which is most, for us) - * will not interrupt the semop(); it will just keep waiting. Therefore - * it's necessary for cancel/die interrupts to be serviced directly by the - * signal handler. On these platforms the behavior is really the same - * whether the signal arrives just before the semop() begins, or while it - * is waiting. The loop on EINTR is thus important only for platforms - * without SA_RESTART. + * We used to check interrupts here, but that required servicing + * interrupts directly from signal handlers. Which is hard to do safely + * and portably. */ do { - ImmediateInterruptOK = interruptOK; - CHECK_FOR_INTERRUPTS(); errStatus = semop(sema->semId, &sops, 1); - ImmediateInterruptOK = false; } while (errStatus < 0 && errno == EINTR); if (errStatus < 0) diff --git a/src/backend/port/win32_sema.c b/src/backend/port/win32_sema.c index f848ff82b09..011e2fd4a6a 100644 --- a/src/backend/port/win32_sema.c +++ b/src/backend/port/win32_sema.c @@ -116,13 +116,11 @@ PGSemaphoreReset(PGSemaphore sema) * Serve the interrupt if interruptOK is true. */ void -PGSemaphoreLock(PGSemaphore sema, bool interruptOK) +PGSemaphoreLock(PGSemaphore sema) { HANDLE wh[2]; bool done = false; - ImmediateInterruptOK = interruptOK; - /* * Note: pgwin32_signal_event should be first to ensure that it will be * reported when multiple events are set. We want to guarantee that @@ -173,8 +171,6 @@ PGSemaphoreLock(PGSemaphore sema, bool interruptOK) break; } } - - ImmediateInterruptOK = false; } /* diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index 7cb002357a2..7ca8dc0007b 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -863,8 +863,7 @@ LWLockDequeueSelf(LWLock *lock) */ for (;;) { - /* "false" means cannot accept cancel/die interrupt here. */ - PGSemaphoreLock(&MyProc->sem, false); + PGSemaphoreLock(&MyProc->sem); if (!MyProc->lwWaiting) break; extraWaits++; @@ -1034,8 +1033,7 @@ LWLockAcquireCommon(LWLock *lock, LWLockMode mode, uint64 *valptr, uint64 val) for (;;) { - /* "false" means cannot accept cancel/die interrupt here. */ - PGSemaphoreLock(&proc->sem, false); + PGSemaphoreLock(&proc->sem); if (!proc->lwWaiting) break; extraWaits++; @@ -1195,8 +1193,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode) for (;;) { - /* "false" means cannot accept cancel/die interrupt here. */ - PGSemaphoreLock(&proc->sem, false); + PGSemaphoreLock(&proc->sem); if (!proc->lwWaiting) break; extraWaits++; @@ -1397,8 +1394,7 @@ LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval) for (;;) { - /* "false" means cannot accept cancel/die interrupt here. */ - PGSemaphoreLock(&proc->sem, false); + PGSemaphoreLock(&proc->sem); if (!proc->lwWaiting) break; extraWaits++; diff --git a/src/include/storage/pg_sema.h b/src/include/storage/pg_sema.h index 5eaf2bf582a..9f8be759fba 100644 --- a/src/include/storage/pg_sema.h +++ b/src/include/storage/pg_sema.h @@ -72,7 +72,7 @@ extern void PGSemaphoreCreate(PGSemaphore sema); extern void PGSemaphoreReset(PGSemaphore sema); /* Lock a semaphore (decrement count), blocking if count would be < 0 */ -extern void PGSemaphoreLock(PGSemaphore sema, bool interruptOK); +extern void PGSemaphoreLock(PGSemaphore sema); /* Unlock a semaphore (increment count) */ extern void PGSemaphoreUnlock(PGSemaphore sema); |