]> granicus.if.org Git - postgresql/commitdiff
Remove the option to service interrupts during PGSemaphoreLock().
authorAndres Freund <andres@anarazel.de>
Tue, 3 Feb 2015 22:25:00 +0000 (23:25 +0100)
committerAndres Freund <andres@anarazel.de>
Tue, 3 Feb 2015 22:25:00 +0000 (23:25 +0100)
The remaining caller (lwlocks) doesn't need that facility, and we plan
to remove ImmedidateInterruptOK entirely. That means that interrupts
can't be serviced race-free and portably anyway, so there's little
reason for keeping the feature.

Reviewed-By: Heikki Linnakangas
src/backend/port/posix_sema.c
src/backend/port/sysv_sema.c
src/backend/port/win32_sema.c
src/backend/storage/lmgr/lwlock.c
src/include/storage/pg_sema.h

index 633bf5afebb0df63a0d84f164f03dd123bc8085b..ad9c80a8b5be7d138d5c9ea35c342627fb0f4f83 100644 (file)
@@ -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)
index 5a0193f06b1c461a48fce7fe1b1889217a6b5ab2..ba46c640fc9a1a70a3ef88ae3bc6582a0e327e38 100644 (file)
@@ -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)
index f848ff82b09ba35a2f4f25256ade6aab15fa13d8..011e2fd4a6ac2a301e6baa45e034f6914bfc0d2f 100644 (file)
@@ -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;
 }
 
 /*
index 7cb002357a26c7495857599cdf1b0e03df864679..7ca8dc0007bd2d77277e7de1ab919b2592f87819 100644 (file)
@@ -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++;
index 5eaf2bf582a7f2c7e8e0bb408c0f1fdb2a0038c3..9f8be759fbafac564d5420f823a1a4cb1a5eb26d 100644 (file)
@@ -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);