diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 174c72a14b..1342c18c74 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -588,7 +588,7 @@ ProcessClientWriteInterrupt(bool blocked) { /* * Don't mess with whereToSendOutput if ProcessInterrupts wouldn't - * do anything. + * service ProcDiePending. */ if (InterruptHoldoffCount == 0 && CritSectionCount == 0) { @@ -3026,6 +3026,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) @@ -3130,7 +3136,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 72e3352398..6e80065374 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 @@ -94,24 +103,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++)