save-one-setitimer.patch

text/x-patch

Filename: save-one-setitimer.patch
Type: text/x-patch
Part: 0
Message: Re: Strange Windows problem, lock_timeout test request

Patch

Format: unified
File+
src/backend/utils/misc/timeout.c 31 41
diff --git a/src/backend/utils/misc/timeout.c b/src/backend/utils/misc/timeout.c
index b5a3c8f..314a601 100644
--- a/src/backend/utils/misc/timeout.c
+++ b/src/backend/utils/misc/timeout.c
@@ -49,52 +49,29 @@ static bool all_timeouts_initialized = false;
 static volatile int num_active_timeouts = 0;
 static timeout_params *volatile active_timeouts[MAX_TIMEOUTS];
 
+/*
+ * Safeguard for the signal handler.
+ */
+static volatile bool alarm_enabled = false;
 
 /*****************************************************************************
  * Internal helper functions
  *
  * For all of these, it is caller's responsibility to protect them from
- * interruption by the signal handler.	Generally, call disable_alarm()
- * first to prevent interruption, then update state, and last call
+ * interruption by the signal handler.	Generally, set alarm_enabled to false
+ * first to prevent interrupt to do anything, then update state, and last call
  * schedule_alarm(), which will re-enable the interrupt if needed.
- *****************************************************************************/
-
-/*
- * Disable alarm interrupts
  *
- * multi_insert must be true if the caller intends to activate multiple new
- * timeouts.  Otherwise it should be false.
- */
-static void
-disable_alarm(bool multi_insert)
-{
-	struct itimerval timeval;
+ * There may be a pending interrupt during enable_timeout_at(),
+ * enable_timeout_after() or enablwe_timeouts(). But since alarm_enabled is
+ * false, it is harmless when it triggers. We will reschedule the interrupt
+ * that just fired. In the worst case, it will be late by one unit of the
+ * the timer resolution in the OS.
+ *
+ *****************************************************************************/
 
-	/*
-	 * If num_active_timeouts is zero, and multi_insert is false, we don't
-	 * have to call setitimer.	There should not be any pending interrupt, and
-	 * even if there is, the worst possible case is that the signal handler
-	 * fires during schedule_alarm.  (If it fires at any point before
-	 * insert_timeout has incremented num_active_timeouts, it will do nothing,
-	 * since it sees no active timeouts.)  In that case we could end up
-	 * scheduling a useless interrupt ... but when the extra interrupt does
-	 * happen, the signal handler will do nothing, so it's all good.
-	 *
-	 * However, if the caller intends to do anything more after first calling
-	 * insert_timeout, the above argument breaks down, since the signal
-	 * handler could interrupt the subsequent operations leading to corrupt
-	 * state.  Out of an abundance of caution, we forcibly disable the timer
-	 * even though it should be off already, just to be sure.  Even though
-	 * this setitimer call is probably useless, we're still ahead of the game
-	 * compared to scheduling two or more timeouts independently.
-	 */
-	if (multi_insert || num_active_timeouts > 0)
-	{
-		MemSet(&timeval, 0, sizeof(struct itimerval));
-		if (setitimer(ITIMER_REAL, &timeval, NULL) != 0)
-			elog(FATAL, "could not disable SIGALRM timer: %m");
-	}
-}
+#define disable_alarm() \
+	do { alarm_enabled = false; } while (0)
 
 /*
  * Find the index of a given timeout reason in the active array.
@@ -228,6 +205,14 @@ schedule_alarm(TimestampTz now)
 		timeval.it_value.tv_sec = secs;
 		timeval.it_value.tv_usec = usecs;
 
+		/*
+		 * Enable the interrupt handler, just before calling setitimer(),
+		 * so in case SIGALRM triggers just after returning from the
+		 * syscall, it won't be a no-op. Use the do { ... } while (0)
+		 * trick so the compiler doesn't reorder this statement.
+		 */
+		do { alarm_enabled = true; } while (0);
+
 		/* Set the alarm timer */
 		if (setitimer(ITIMER_REAL, &timeval, NULL) != 0)
 			elog(FATAL, "could not enable SIGALRM timer: %m");
@@ -248,7 +233,12 @@ schedule_alarm(TimestampTz now)
 static void
 handle_sig_alarm(SIGNAL_ARGS)
 {
-	int			save_errno = errno;
+	int			save_errno;
+
+	if (!alarm_enabled)
+		return;
+
+	save_errno = errno;
 
 	/*
 	 * SIGALRM is always cause for waking anything waiting on the process
@@ -376,7 +366,7 @@ enable_timeout_after(TimeoutId id, int delay_ms)
 	TimestampTz fin_time;
 
 	/* Disable timeout interrupts for safety. */
-	disable_alarm(false);
+	disable_alarm();
 
 	/* Queue the timeout at the appropriate time. */
 	now = GetCurrentTimestamp();
@@ -424,7 +414,7 @@ enable_timeouts(const EnableTimeoutParams *timeouts, int count)
 	int			i;
 
 	/* Disable timeout interrupts for safety. */
-	disable_alarm(count > 1);
+	disable_alarm();
 
 	/* Queue the timeout(s) at the appropriate times. */
 	now = GetCurrentTimestamp();