>From 63078f8d273b6a2dc47bfab093d42b110e2487e5 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Thu, 15 Jan 2015 00:57:12 +0100 Subject: [PATCH 7/7] Remove remnants of ImmediateInterruptOK handling. Now that nothing sets ImmediateInterruptOK to true anymore, we can remove all the supporting code. --- src/backend/storage/ipc/ipc.c | 2 -- src/backend/tcop/postgres.c | 29 -------------------------- src/backend/utils/error/elog.c | 29 +++----------------------- src/backend/utils/init/globals.c | 1 - src/backend/utils/misc/timeout.c | 38 +++-------------------------------- src/include/miscadmin.h | 1 - src/test/modules/test_shm_mq/worker.c | 6 +----- 7 files changed, 7 insertions(+), 99 deletions(-) diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c index f0f7939..6bc0b06 100644 --- a/src/backend/storage/ipc/ipc.c +++ b/src/backend/storage/ipc/ipc.c @@ -165,8 +165,6 @@ proc_exit_prepare(int code) InterruptPending = false; ProcDiePending = false; QueryCancelPending = false; - /* And let's just make *sure* we're not interrupted ... */ - ImmediateInterruptOK = false; InterruptHoldoffCount = 1; CritSectionCount = 0; diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 832561e..bf9832a 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -2648,13 +2648,6 @@ die(SIGNAL_ARGS) { InterruptPending = true; ProcDiePending = true; - - /* - * If we're waiting for input or a lock so that it's safe to - * interrupt, service the interrupt immediately - */ - if (ImmediateInterruptOK) - ProcessInterrupts(); } /* If we're still here, waken anything waiting on the process latch */ @@ -2686,13 +2679,6 @@ StatementCancelHandler(SIGNAL_ARGS) { InterruptPending = true; QueryCancelPending = true; - - /* - * If we're waiting for input or a lock so that it's safe to - * interrupt, service the interrupt immediately - */ - if (ImmediateInterruptOK) - ProcessInterrupts(); } /* If we're still here, waken anything waiting on the process latch */ @@ -2833,13 +2819,6 @@ RecoveryConflictInterrupt(ProcSignalReason reason) */ if (reason == PROCSIG_RECOVERY_CONFLICT_DATABASE) RecoveryConflictRetryable = false; - - /* - * If we're waiting for input or a lock so that it's safe to - * interrupt, service the interrupt immediately. - */ - if (ImmediateInterruptOK) - ProcessInterrupts(); } /* @@ -2873,7 +2852,6 @@ ProcessInterrupts(void) { ProcDiePending = false; QueryCancelPending = false; /* ProcDie trumps QueryCancel */ - ImmediateInterruptOK = false; /* not idle anymore */ LockErrorCleanup(); /* As in quickdie, don't risk sending to client during auth */ if (ClientAuthInProgress && whereToSendOutput == DestRemote) @@ -2912,7 +2890,6 @@ ProcessInterrupts(void) if (ClientConnectionLost) { QueryCancelPending = false; /* lost connection trumps QueryCancel */ - ImmediateInterruptOK = false; /* not idle anymore */ LockErrorCleanup(); /* don't send to client, we already know the connection to be dead. */ whereToSendOutput = DestNone; @@ -2930,7 +2907,6 @@ ProcessInterrupts(void) if (RecoveryConflictPending && DoingCommandRead) { QueryCancelPending = false; /* this trumps QueryCancel */ - ImmediateInterruptOK = false; /* not idle anymore */ RecoveryConflictPending = false; LockErrorCleanup(); pgstat_report_recovery_conflict(RecoveryConflictReason); @@ -2968,7 +2944,6 @@ ProcessInterrupts(void) */ if (get_timeout_indicator(LOCK_TIMEOUT, true)) { - ImmediateInterruptOK = false; /* not idle anymore */ (void) get_timeout_indicator(STATEMENT_TIMEOUT, true); LockErrorCleanup(); ereport(ERROR, @@ -2977,7 +2952,6 @@ ProcessInterrupts(void) } if (get_timeout_indicator(STATEMENT_TIMEOUT, true)) { - ImmediateInterruptOK = false; /* not idle anymore */ LockErrorCleanup(); ereport(ERROR, (errcode(ERRCODE_QUERY_CANCELED), @@ -2985,7 +2959,6 @@ ProcessInterrupts(void) } if (IsAutoVacuumWorkerProcess()) { - ImmediateInterruptOK = false; /* not idle anymore */ LockErrorCleanup(); ereport(ERROR, (errcode(ERRCODE_QUERY_CANCELED), @@ -2993,7 +2966,6 @@ ProcessInterrupts(void) } if (RecoveryConflictPending) { - ImmediateInterruptOK = false; /* not idle anymore */ RecoveryConflictPending = false; LockErrorCleanup(); pgstat_report_recovery_conflict(RecoveryConflictReason); @@ -3010,7 +2982,6 @@ ProcessInterrupts(void) */ if (!DoingCommandRead) { - ImmediateInterruptOK = false; /* not idle anymore */ LockErrorCleanup(); ereport(ERROR, (errcode(ERRCODE_QUERY_CANCELED), diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 0f7aa19..4e0cc30 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -412,7 +412,6 @@ errfinish(int dummy,...) { ErrorData *edata = &errordata[errordata_stack_depth]; int elevel; - bool save_ImmediateInterruptOK; MemoryContext oldcontext; ErrorContextCallback *econtext; @@ -421,18 +420,6 @@ errfinish(int dummy,...) elevel = edata->elevel; /* - * Ensure we can't get interrupted while performing error reporting. This - * is needed to prevent recursive entry to functions like syslog(), which - * may not be re-entrant. - * - * Note: other places that save-and-clear ImmediateInterruptOK also do - * HOLD_INTERRUPTS(), but that should not be necessary here since we don't - * call anything that could turn on ImmediateInterruptOK. - */ - save_ImmediateInterruptOK = ImmediateInterruptOK; - ImmediateInterruptOK = false; - - /* * Do processing in ErrorContext, which we hope has enough reserved space * to report an error. */ @@ -463,10 +450,6 @@ errfinish(int dummy,...) * itself be inside a holdoff section. If necessary, such a handler * could save and restore InterruptHoldoffCount for itself, but this * should make life easier for most.) - * - * Note that we intentionally don't restore ImmediateInterruptOK here, - * even if it was set at entry. We definitely don't want that on - * while doing error cleanup. */ InterruptHoldoffCount = 0; QueryCancelHoldoffCount = 0; @@ -573,15 +556,9 @@ errfinish(int dummy,...) } /* - * We reach here if elevel <= WARNING. OK to return to caller, so restore - * caller's setting of ImmediateInterruptOK. - */ - ImmediateInterruptOK = save_ImmediateInterruptOK; - - /* - * But check for cancel/die interrupt first --- this is so that the user - * can stop a query emitting tons of notice or warning messages, even if - * it's in a loop that otherwise fails to check for interrupts. + * Check for cancel/die interrupt first --- this is so that the user can + * stop a query emitting tons of notice or warning messages, even if it's + * in a loop that otherwise fails to check for interrupts. */ CHECK_FOR_INTERRUPTS(); } diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index 8cf2ead..23e594e 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -30,7 +30,6 @@ volatile bool InterruptPending = false; volatile bool QueryCancelPending = false; volatile bool ProcDiePending = false; volatile bool ClientConnectionLost = false; -volatile bool ImmediateInterruptOK = false; volatile uint32 InterruptHoldoffCount = 0; volatile uint32 QueryCancelHoldoffCount = 0; volatile uint32 CritSectionCount = 0; diff --git a/src/backend/utils/misc/timeout.c b/src/backend/utils/misc/timeout.c index ce4bc13..a7ec665 100644 --- a/src/backend/utils/misc/timeout.c +++ b/src/backend/utils/misc/timeout.c @@ -259,27 +259,12 @@ static void handle_sig_alarm(SIGNAL_ARGS) { int save_errno = errno; - bool save_ImmediateInterruptOK = ImmediateInterruptOK; /* - * We may be executing while ImmediateInterruptOK is true (e.g., when - * mainline is waiting for a lock). If SIGINT or similar arrives while - * this code is running, we'd lose control and perhaps leave our data - * structures in an inconsistent state. Disable immediate interrupts, and - * just to be real sure, bump the holdoff counter as well. (The reason - * for this belt-and-suspenders-too approach is to make sure that nothing - * bad happens if a timeout handler calls code that manipulates - * ImmediateInterruptOK.) - * - * Note: it's possible for a SIGINT to interrupt handle_sig_alarm before - * we manage to do this; the net effect would be as if the SIGALRM event - * had been silently lost. Therefore error recovery must include some - * action that will allow any lost interrupt to be rescheduled. Disabling - * some or all timeouts is sufficient, or if that's not appropriate, - * reschedule_timeouts() can be called. Also, the signal blocking hazard - * described below applies here too. + * Bump the holdoff counter, to make sure nothing we call will process + * interrupts directly. No timeout handler should do that, but these + * failures are hard to debug, so better be sure. */ - ImmediateInterruptOK = false; HOLD_INTERRUPTS(); /* @@ -332,24 +317,7 @@ handle_sig_alarm(SIGNAL_ARGS) } } - /* - * Re-allow query cancel, and then try to service any cancel request that - * arrived meanwhile (this might in particular include a cancel request - * fired by one of the timeout handlers). Since we are in a signal - * handler, we mustn't call ProcessInterrupts unless ImmediateInterruptOK - * is set; if it isn't, the cancel will happen at the next mainline - * CHECK_FOR_INTERRUPTS. - * - * Note: a longjmp from here is safe so far as our own data structures are - * concerned; but on platforms that block a signal before calling the - * handler and then un-block it on return, longjmping out of the signal - * handler leaves SIGALRM still blocked. Error cleanup is responsible for - * unblocking any blocked signals. - */ RESUME_INTERRUPTS(); - ImmediateInterruptOK = save_ImmediateInterruptOK; - if (save_ImmediateInterruptOK) - CHECK_FOR_INTERRUPTS(); errno = save_errno; } diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index c9a46aa..eacfccb 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -84,7 +84,6 @@ extern PGDLLIMPORT volatile bool ProcDiePending; extern volatile bool ClientConnectionLost; /* these are marked volatile because they are examined by signal handlers: */ -extern PGDLLIMPORT volatile bool ImmediateInterruptOK; extern PGDLLIMPORT volatile uint32 InterruptHoldoffCount; extern PGDLLIMPORT volatile uint32 QueryCancelHoldoffCount; extern PGDLLIMPORT volatile uint32 CritSectionCount; diff --git a/src/test/modules/test_shm_mq/worker.c b/src/test/modules/test_shm_mq/worker.c index a9d9e0e..691a568 100644 --- a/src/test/modules/test_shm_mq/worker.c +++ b/src/test/modules/test_shm_mq/worker.c @@ -60,13 +60,9 @@ test_shm_mq_main(Datum main_arg) * * We want CHECK_FOR_INTERRUPTS() to kill off this worker process just as * it would a normal user backend. To make that happen, we establish a - * signal handler that is a stripped-down version of die(). We don't have - * any equivalent of the backend's command-read loop, where interrupts can - * be processed immediately, so make sure ImmediateInterruptOK is turned - * off. + * signal handler that is a stripped-down version of die(). */ pqsignal(SIGTERM, handle_sigterm); - ImmediateInterruptOK = false; BackgroundWorkerUnblockSignals(); /* -- 2.0.0.rc2.4.g1dc51c6.dirty