Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 4c646b1

Browse files
committed
Refactor CHECK_FOR_INTERRUPTS() to add flexibility.
Split up CHECK_FOR_INTERRUPTS() to provide an additional macro INTERRUPTS_PENDING_CONDITION(), which just tests whether an interrupt is pending without attempting to service it. This is useful in situations where the caller knows that interrupts are blocked, and would like to find out if it's worth the trouble to unblock them. Also add INTERRUPTS_CAN_BE_PROCESSED(), which indicates whether CHECK_FOR_INTERRUPTS() can be relied on to clear the pending interrupt. This commit doesn't actually add any uses of the new macros, but a follow-on bug fix will do so. Back-patch to all supported branches to provide infrastructure for that fix. Alvaro Herrera and Tom Lane Discussion: https://postgr.es/m/20210513155351.GA7848@alvherre.pgsql
1 parent f0d20e5 commit 4c646b1

File tree

2 files changed

+35
-13
lines changed

2 files changed

+35
-13
lines changed

src/backend/tcop/postgres.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,7 @@ ProcessClientWriteInterrupt(bool blocked)
592592
{
593593
/*
594594
* Don't mess with whereToSendOutput if ProcessInterrupts wouldn't
595-
* do anything.
595+
* service ProcDiePending.
596596
*/
597597
if (InterruptHoldoffCount == 0 && CritSectionCount == 0)
598598
{
@@ -2959,6 +2959,12 @@ RecoveryConflictInterrupt(ProcSignalReason reason)
29592959
* If an interrupt condition is pending, and it's safe to service it,
29602960
* then clear the flag and accept the interrupt. Called only when
29612961
* InterruptPending is true.
2962+
*
2963+
* Note: if INTERRUPTS_CAN_BE_PROCESSED() is true, then ProcessInterrupts
2964+
* is guaranteed to clear the InterruptPending flag before returning.
2965+
* (This is not the same as guaranteeing that it's still clear when we
2966+
* return; another interrupt could have arrived. But we promise that
2967+
* any pre-existing one will have been serviced.)
29622968
*/
29632969
void
29642970
ProcessInterrupts(void)
@@ -3063,7 +3069,11 @@ ProcessInterrupts(void)
30633069
{
30643070
/*
30653071
* Re-arm InterruptPending so that we process the cancel request as
3066-
* soon as we're done reading the message.
3072+
* soon as we're done reading the message. (XXX this is seriously
3073+
* ugly: it complicates INTERRUPTS_CAN_BE_PROCESSED(), and it means we
3074+
* can't use that macro directly as the initial test in this function,
3075+
* meaning that this code also creates opportunities for other bugs to
3076+
* appear.)
30673077
*/
30683078
InterruptPending = true;
30693079
}

src/include/miscadmin.h

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,15 @@
5757
* allowing die interrupts: HOLD_CANCEL_INTERRUPTS() and
5858
* RESUME_CANCEL_INTERRUPTS().
5959
*
60+
* Note that ProcessInterrupts() has also acquired a number of tasks that
61+
* do not necessarily cause a query-cancel-or-die response. Hence, it's
62+
* possible that it will just clear InterruptPending and return.
63+
*
64+
* INTERRUPTS_PENDING_CONDITION() can be checked to see whether an
65+
* interrupt needs to be serviced, without trying to do so immediately.
66+
* Some callers are also interested in INTERRUPTS_CAN_BE_PROCESSED(),
67+
* which tells whether ProcessInterrupts is sure to clear the interrupt.
68+
*
6069
* Special mechanisms are used to let an interrupt be accepted when we are
6170
* waiting for a lock or when we are waiting for command input (but, of
6271
* course, only if the interrupt holdoff counter is zero). See the
@@ -94,24 +103,27 @@ extern PGDLLIMPORT volatile uint32 CritSectionCount;
94103
/* in tcop/postgres.c */
95104
extern void ProcessInterrupts(void);
96105

106+
/* Test whether an interrupt is pending */
97107
#ifndef WIN32
108+
#define INTERRUPTS_PENDING_CONDITION() \
109+
(unlikely(InterruptPending))
110+
#else
111+
#define INTERRUPTS_PENDING_CONDITION() \
112+
(unlikely(UNBLOCKED_SIGNAL_QUEUE()) ? pgwin32_dispatch_queued_signals() : 0, \
113+
unlikely(InterruptPending))
114+
#endif
98115

116+
/* Service interrupt, if one is pending and it's safe to service it now */
99117
#define CHECK_FOR_INTERRUPTS() \
100118
do { \
101-
if (unlikely(InterruptPending)) \
102-
ProcessInterrupts(); \
103-
} while(0)
104-
#else /* WIN32 */
105-
106-
#define CHECK_FOR_INTERRUPTS() \
107-
do { \
108-
if (unlikely(UNBLOCKED_SIGNAL_QUEUE())) \
109-
pgwin32_dispatch_queued_signals(); \
110-
if (unlikely(InterruptPending)) \
119+
if (INTERRUPTS_PENDING_CONDITION()) \
111120
ProcessInterrupts(); \
112121
} while(0)
113-
#endif /* WIN32 */
114122

123+
/* Is ProcessInterrupts() guaranteed to clear InterruptPending? */
124+
#define INTERRUPTS_CAN_BE_PROCESSED() \
125+
(InterruptHoldoffCount == 0 && CritSectionCount == 0 && \
126+
QueryCancelHoldoffCount == 0)
115127

116128
#define HOLD_INTERRUPTS() (InterruptHoldoffCount++)
117129

0 commit comments

Comments
 (0)