diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 6200699ddd7..dd2ade7bb65 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -554,7 +554,7 @@ ProcessClientWriteInterrupt(bool blocked) { /* * Don't mess with whereToSendOutput if ProcessInterrupts wouldn't - * do anything. + * service ProcDiePending. */ if (InterruptHoldoffCount == 0 && CritSectionCount == 0) { @@ -3118,6 +3118,12 @@ RecoveryConflictInterrupt(ProcSignalReason reason) * If an interrupt condition is pending, and it's safe to service it, * then clear the flag and accept the interrupt. Called only when * InterruptPending is true. + * + * Note: if INTERRUPTS_CAN_BE_PROCESSED() is true, then ProcessInterrupts + * is guaranteed to clear the InterruptPending flag before returning. + * (This is not the same as guaranteeing that it's still clear when we + * return; another interrupt could have arrived. But we promise that + * any pre-existing one will have been serviced.) */ void ProcessInterrupts(void) @@ -3248,7 +3254,11 @@ ProcessInterrupts(void) { /* * Re-arm InterruptPending so that we process the cancel request as - * soon as we're done reading the message. + * soon as we're done reading the message. (XXX this is seriously + * ugly: it complicates INTERRUPTS_CAN_BE_PROCESSED(), and it means we + * can't use that macro directly as the initial test in this function, + * meaning that this code also creates opportunities for other bugs to + * appear.) */ InterruptPending = true; } diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 95202d37af5..4dc343cbc59 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -57,6 +57,15 @@ * allowing die interrupts: HOLD_CANCEL_INTERRUPTS() and * RESUME_CANCEL_INTERRUPTS(). * + * Note that ProcessInterrupts() has also acquired a number of tasks that + * do not necessarily cause a query-cancel-or-die response. Hence, it's + * possible that it will just clear InterruptPending and return. + * + * INTERRUPTS_PENDING_CONDITION() can be checked to see whether an + * interrupt needs to be serviced, without trying to do so immediately. + * Some callers are also interested in INTERRUPTS_CAN_BE_PROCESSED(), + * which tells whether ProcessInterrupts is sure to clear the interrupt. + * * Special mechanisms are used to let an interrupt be accepted when we are * waiting for a lock or when we are waiting for command input (but, of * course, only if the interrupt holdoff counter is zero). See the @@ -97,24 +106,27 @@ extern PGDLLIMPORT volatile uint32 CritSectionCount; /* in tcop/postgres.c */ extern void ProcessInterrupts(void); +/* Test whether an interrupt is pending */ #ifndef WIN32 +#define INTERRUPTS_PENDING_CONDITION() \ + (unlikely(InterruptPending)) +#else +#define INTERRUPTS_PENDING_CONDITION() \ + (unlikely(UNBLOCKED_SIGNAL_QUEUE()) ? pgwin32_dispatch_queued_signals() : 0, \ + unlikely(InterruptPending)) +#endif +/* Service interrupt, if one is pending and it's safe to service it now */ #define CHECK_FOR_INTERRUPTS() \ do { \ - if (unlikely(InterruptPending)) \ + if (INTERRUPTS_PENDING_CONDITION()) \ ProcessInterrupts(); \ } while(0) -#else /* WIN32 */ - -#define CHECK_FOR_INTERRUPTS() \ -do { \ - if (unlikely(UNBLOCKED_SIGNAL_QUEUE())) \ - pgwin32_dispatch_queued_signals(); \ - if (unlikely(InterruptPending)) \ - ProcessInterrupts(); \ -} while(0) -#endif /* WIN32 */ +/* Is ProcessInterrupts() guaranteed to clear InterruptPending? */ +#define INTERRUPTS_CAN_BE_PROCESSED() \ + (InterruptHoldoffCount == 0 && CritSectionCount == 0 && \ + QueryCancelHoldoffCount == 0) #define HOLD_INTERRUPTS() (InterruptHoldoffCount++)