save-one-setitimer.patch
text/x-patch
Filename: save-one-setitimer.patch
Type: text/x-patch
Part: 0
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();