database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-04-29 13:56:47 +03:00
Code

Make timeout.c more robust against missed timer interrupts.

Browse Source 
Commit 09cf1d522 taught schedule_alarm() to not do anything if
the next requested event is after when we expect the next interrupt
to fire.  However, if somehow an interrupt gets lost, we'll continue
to not do anything indefinitely, even after the "next interrupt" time
is obviously in the past.  Thus, one missed interrupt can break
timeout scheduling for the life of the session.  Michael Harris
reported a scenario where a bug in a user-defined function caused this
to happen, so you don't even need to assume kernel bugs exist to think
this is worth fixing.  We can make things more robust at little cost
by detecting the case where signal_due_at is before "now" and forcing
a new setitimer call to occur.  This isn't a completely bulletproof
fix of course; but in our typical usage pattern where we frequently set
timeouts and clear them before they are reached, the interrupt will
get re-enabled after at most one timeout interval, which with a little
luck will be before we really need it.

While here, let's mark signal_due_at as volatile, since the signal
handler can both examine and set it.  I'm not sure there's any
actual risk given that signal_pending is already volatile, but
it's surely questionable.

Backpatch to v14 where this logic came in.

Michael Harris and Tom Lane

Discussion: https://postgr.es/m/CADofcAWbMrvgwSMqO4iG_iD3E2v8ZUrC-_crB41my=VMM02-CA@mail.gmail.com
This commit is contained in:
Tom Lane 2022-02-10 11:52:20 -05:00
parent 5f00ef065e
commit 2e211c1661
1 changed files with 16 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/backend/utils/misc/timeout.c
View File

@ -70,11 +70,12 @@ static volatile sig_atomic_t alarm_enabled = false;
/*
* State recording if and when we next expect the interrupt to fire.
* (signal_due_at is valid only when signal_pending is true.)
* Note that the signal handler will unconditionally reset signal_pending to
* false, so that can change asynchronously even when alarm_enabled is false.
*/
static volatile sig_atomic_t signal_pending = false;
static TimestampTz signal_due_at = 0; /* valid only when signal_pending */
static volatile TimestampTz signal_due_at = 0;
/*****************************************************************************
@ -214,10 +215,22 @@ schedule_alarm(TimestampTz now)
MemSet(&timeval, 0, sizeof(struct itimerval));
/*
* If we think there's a signal pending, but current time is more than
* 10ms past when the signal was due, then assume that the timeout
* request got lost somehow; clear signal_pending so that we'll reset
* the interrupt request below. (10ms corresponds to the worst-case
* timeout granularity on modern systems.) It won't hurt us if the
* interrupt does manage to fire between now and when we reach the
* setitimer() call.
*/
if (signal_pending && now > signal_due_at + 10 * 1000)
signal_pending = false;
/*
* Get the time remaining till the nearest pending timeout. If it is
* negative, assume that we somehow missed an interrupt, and force
* signal_pending off. This gives us a chance to recover if the
* negative, assume that we somehow missed an interrupt, and clear
* signal_pending. This gives us another chance to recover if the
* kernel drops a timeout request for some reason.
*/
nearest_timeout = active_timeouts[0]->fin_time;