From 2efc6dc256dc71ee876304b51dcad29fafbc37d3 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 13 Dec 2013 14:05:14 -0500 Subject: [PATCH] Add HOLD/RESUME_INTERRUPTS in HandleCatchupInterrupt/HandleNotifyInterrupt. This prevents a possible longjmp out of the signal handler if a timeout or SIGINT occurs while something within the handler has transiently set ImmediateInterruptOK. For safety we must hold off the timeout or cancel error until we're back in mainline, or at least till we reach the end of the signal handler when ImmediateInterruptOK was true at entry. This syncs these functions with the logic now present in handle_sig_alarm. AFAICT there is no live bug here in 9.0 and up, because I don't think we currently can wait for any heavyweight lock inside these functions, and there is no other code (except read-from-client) that will turn on ImmediateInterruptOK. However, that was not true pre-9.0: in older branches ProcessIncomingNotify might block trying to lock pg_listener, and then a SIGINT could lead to undesirable control flow. It might be all right anyway given the relatively narrow code ranges in which NOTIFY interrupts are enabled, but for safety's sake I'm back-patching this. --- src/backend/commands/async.c | 14 ++++++++++---- src/backend/storage/ipc/sinval.c | 14 ++++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 6414291ef7..f4526a6c4d 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -1650,11 +1650,15 @@ HandleNotifyInterrupt(void) /* * We may be called while ImmediateInterruptOK is true; turn it off - * while messing with the NOTIFY state. (We would have to save and - * restore it anyway, because PGSemaphore operations inside - * ProcessIncomingNotify() might reset it.) + * while messing with the NOTIFY state. This prevents problems if + * SIGINT or similar arrives while we're working. Just to be real + * sure, bump the interrupt holdoff counter as well. That way, even + * if something inside ProcessIncomingNotify() transiently sets + * ImmediateInterruptOK (eg while waiting on a lock), we won't get + * interrupted until we're done with the notify interrupt. */ ImmediateInterruptOK = false; + HOLD_INTERRUPTS(); /* * I'm not sure whether some flavors of Unix might allow another @@ -1684,8 +1688,10 @@ HandleNotifyInterrupt(void) } /* - * Restore ImmediateInterruptOK, and check for interrupts if needed. + * Restore the holdoff level and ImmediateInterruptOK, and check for + * interrupts if needed. */ + RESUME_INTERRUPTS(); ImmediateInterruptOK = save_ImmediateInterruptOK; if (save_ImmediateInterruptOK) CHECK_FOR_INTERRUPTS(); diff --git a/src/backend/storage/ipc/sinval.c b/src/backend/storage/ipc/sinval.c index 5e48d01178..98eb814dc3 100644 --- a/src/backend/storage/ipc/sinval.c +++ b/src/backend/storage/ipc/sinval.c @@ -180,11 +180,15 @@ HandleCatchupInterrupt(void) /* * We may be called while ImmediateInterruptOK is true; turn it off - * while messing with the catchup state. (We would have to save and - * restore it anyway, because PGSemaphore operations inside - * ProcessCatchupEvent() might reset it.) + * while messing with the catchup state. This prevents problems if + * SIGINT or similar arrives while we're working. Just to be real + * sure, bump the interrupt holdoff counter as well. That way, even + * if something inside ProcessCatchupEvent() transiently sets + * ImmediateInterruptOK (eg while waiting on a lock), we won't get + * interrupted until we're done with the catchup interrupt. */ ImmediateInterruptOK = false; + HOLD_INTERRUPTS(); /* * I'm not sure whether some flavors of Unix might allow another @@ -208,8 +212,10 @@ HandleCatchupInterrupt(void) } /* - * Restore ImmediateInterruptOK, and check for interrupts if needed. + * Restore the holdoff level and ImmediateInterruptOK, and check for + * interrupts if needed. */ + RESUME_INTERRUPTS(); ImmediateInterruptOK = save_ImmediateInterruptOK; if (save_ImmediateInterruptOK) CHECK_FOR_INTERRUPTS();