Remove remnants of ImmediateInterruptOK handling.

anarazel · anarazel · commit 2505ce0be0b6 · 2015-02-03T23:25:47.000+01:00
Now that nothing sets ImmediateInterruptOK to true anymore, we can
remove all the supporting code.

Reviewed-By: Heikki Linnakangas
diff --git 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
@@ -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 */
@@ -2688,13 +2681,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 */
@@ -2835,13 +2821,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();
 	}
 
 	/*
@@ -2875,7 +2854,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)
@@ -2914,7 +2892,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;
@@ -2932,7 +2909,6 @@ ProcessInterrupts(void)
 	if (RecoveryConflictPending && DoingCommandRead)
 	{
 		QueryCancelPending = false;			/* this trumps QueryCancel */
-		ImmediateInterruptOK = false;		/* not idle anymore */
 		RecoveryConflictPending = false;
 		LockErrorCleanup();
 		pgstat_report_recovery_conflict(RecoveryConflictReason);
@@ -2970,7 +2946,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,
@@ -2979,23 +2954,20 @@ ProcessInterrupts(void)
 		}
 		if (get_timeout_indicator(STATEMENT_TIMEOUT, true))
 		{
-			ImmediateInterruptOK = false;		/* not idle anymore */
 			LockErrorCleanup();
 			ereport(ERROR,
 					(errcode(ERRCODE_QUERY_CANCELED),
 					 errmsg("canceling statement due to statement timeout")));
 		}
 		if (IsAutoVacuumWorkerProcess())
 		{
-			ImmediateInterruptOK = false;		/* not idle anymore */
 			LockErrorCleanup();
 			ereport(ERROR,
 					(errcode(ERRCODE_QUERY_CANCELED),
 					 errmsg("canceling autovacuum task")));
 		}
 		if (RecoveryConflictPending)
 		{
-			ImmediateInterruptOK = false;		/* not idle anymore */
 			RecoveryConflictPending = false;
 			LockErrorCleanup();
 			pgstat_report_recovery_conflict(RecoveryConflictReason);
@@ -3012,7 +2984,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
@@ -412,26 +412,13 @@ errfinish(int dummy,...)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
 	int			elevel;
-	bool		save_ImmediateInterruptOK;
 	MemoryContext oldcontext;
 	ErrorContextCallback *econtext;
 
 	recursion_depth++;
 	CHECK_STACK_DEPTH();
 	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
@@ -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
@@ -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
@@ -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
@@ -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();
 
 	/*
