diff --git a/src/backend/port/posix_sema.c b/src/backend/port/posix_sema.c index 7eaa89cdddb57bfeed2e29595ae3614c2e0ffd70..335dae270d0595013fed5711ec5d875545b10edc 100644 --- a/src/backend/port/posix_sema.c +++ b/src/backend/port/posix_sema.c @@ -11,7 +11,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/port/posix_sema.c,v 1.19 2008/01/01 19:45:51 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/port/posix_sema.c,v 1.20 2008/01/26 19:55:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -241,37 +241,10 @@ PGSemaphoreLock(PGSemaphore sema, bool interruptOK) int errStatus; /* - * Note: if errStatus is -1 and errno == EINTR then it means we returned - * 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. We - * assume that if such an interrupt comes in while we are waiting, it will - * cause the sem_wait() call to exit with errno == EINTR, so that we will - * be able to service the interrupt (if not in a critical section - * already). - * - * 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 sem_wait() 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 sem_wait()). 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 sem_wait() 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 Posix semaphores - * used to implement LW locks or emulate spinlocks --- but the wait time - * for such locks should not be very long, anyway.) + * 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. */ do { diff --git a/src/backend/port/sysv_sema.c b/src/backend/port/sysv_sema.c index 876b9c6bbb03b09f210ba5b5303ed4c702d544ab..54a1f7c93b388e9e2085ac91ed15240bc0e7cdda 100644 --- a/src/backend/port/sysv_sema.c +++ b/src/backend/port/sysv_sema.c @@ -8,7 +8,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/port/sysv_sema.c,v 1.22 2008/01/01 19:45:51 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/port/sysv_sema.c,v 1.23 2008/01/26 19:55:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -377,10 +377,11 @@ 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. We - * assume that if such an interrupt comes in while we are waiting, it will - * cause the semop() call to exit with errno == EINTR, so that we will be - * able to service the interrupt (if not in a critical section already). + * 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 @@ -403,6 +404,14 @@ PGSemaphoreLock(PGSemaphore sema, bool interruptOK) * 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 other types + * of interrupts. */ do { diff --git a/src/backend/port/win32_sema.c b/src/backend/port/win32_sema.c index 6dc50bff8d64e9a1525645233fc3458bde4c7d40..3c2248b0788e3de489ed28f8a9a23a9626be3059 100644 --- a/src/backend/port/win32_sema.c +++ b/src/backend/port/win32_sema.c @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/port/win32_sema.c,v 1.6 2008/01/01 19:45:51 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/port/win32_sema.c,v 1.7 2008/01/26 19:55:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -124,6 +124,12 @@ PGSemaphoreLock(PGSemaphore sema, bool interruptOK) wh[0] = *sema; wh[1] = pgwin32_signal_event; + /* + * As in other implementations of PGSemaphoreLock, we need to check + * for cancel/die interrupts each time through the loop. But here, + * there is no hidden magic about whether the syscall will internally + * service a signal --- we do that ourselves. + */ do { ImmediateInterruptOK = interruptOK; diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 48d18e54781d5134c73203d638f9b8ca4b454884..0ac70da1f0a01293039a07aebf80b997fefe9321 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/lmgr/proc.c,v 1.198 2008/01/01 19:45:52 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/storage/lmgr/proc.c,v 1.199 2008/01/26 19:55:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -486,20 +486,18 @@ HaveNFreeProcs(int n) /* * Cancel any pending wait for lock, when aborting a transaction. * - * Returns true if we had been waiting for a lock, else false. - * * (Normally, this would only happen if we accept a cancel/die * interrupt while waiting; but an ereport(ERROR) while waiting is * within the realm of possibility, too.) */ -bool +void LockWaitCancel(void) { LWLockId partitionLock; /* Nothing to do if we weren't waiting for a lock */ if (lockAwaited == NULL) - return false; + return; /* Turn off the deadlock timer, if it's still running (see ProcSleep) */ disable_sig_alarm(false); @@ -538,12 +536,6 @@ LockWaitCancel(void) * wakeup signal isn't harmful, and it seems not worth expending cycles to * get rid of a signal that most likely isn't there. */ - - /* - * Return true even if we were kicked off the lock before we were able to - * remove ourselves. - */ - return true; } diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 05127c40b309a7d82a186f6ceb38fe7c00c81158..325411df09ca2e3b96c1da6e06d901570d3f67e8 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.541 2008/01/01 19:45:52 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.542 2008/01/26 19:55:08 tgl Exp $ * * NOTES * this is the "main" module of the postgres backend and @@ -2449,10 +2449,9 @@ die(SIGNAL_ARGS) /* bump holdoff count to make ProcessInterrupts() a no-op */ /* until we are done getting ready for it */ InterruptHoldoffCount++; + LockWaitCancel(); /* prevent CheckDeadLock from running */ DisableNotifyInterrupt(); DisableCatchupInterrupt(); - /* Make sure CheckDeadLock won't run while shutting down... */ - LockWaitCancel(); InterruptHoldoffCount--; ProcessInterrupts(); } @@ -2498,20 +2497,16 @@ StatementCancelHandler(SIGNAL_ARGS) * waiting for input, however. */ if (ImmediateInterruptOK && InterruptHoldoffCount == 0 && - CritSectionCount == 0) + CritSectionCount == 0 && !DoingCommandRead) { /* bump holdoff count to make ProcessInterrupts() a no-op */ /* until we are done getting ready for it */ InterruptHoldoffCount++; - if (LockWaitCancel()) - { - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); - InterruptHoldoffCount--; - ProcessInterrupts(); - } - else - InterruptHoldoffCount--; + LockWaitCancel(); /* prevent CheckDeadLock from running */ + DisableNotifyInterrupt(); + DisableCatchupInterrupt(); + InterruptHoldoffCount--; + ProcessInterrupts(); } } diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 5b7c7ec4e03fd7f077eb9d5353f252e3e0789c7a..1ce3eb26fc324a49ff144c8af155402ef5e174d5 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/storage/proc.h,v 1.103 2008/01/01 19:45:59 momjian Exp $ + * $PostgreSQL: pgsql/src/include/storage/proc.h,v 1.104 2008/01/26 19:55:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -166,7 +166,7 @@ extern void ProcQueueInit(PROC_QUEUE *queue); extern int ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable); extern PGPROC *ProcWakeup(PGPROC *proc, int waitStatus); extern void ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock); -extern bool LockWaitCancel(void); +extern void LockWaitCancel(void); extern void ProcWaitForSignal(void); extern void ProcSendSignal(int pid);