From b8d0cda53377515ac61357ec4a60e85ca873f486 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 7 Jan 2021 11:45:08 -0500 Subject: [PATCH] Further second thoughts about idle_session_timeout patch. On reflection, the order of operations in PostgresMain() is wrong. These timeouts ought to be shut down before, not after, we do the post-command-read CHECK_FOR_INTERRUPTS, to guarantee that any timeout error will be detected there rather than at some ill-defined later point (possibly after having wasted a lot of work). This is really an error in the original idle_in_transaction_timeout patch, so back-patch to 9.6 where that was introduced. --- src/backend/tcop/postgres.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 2b53ebf97d..28055680aa 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -4323,22 +4323,11 @@ PostgresMain(int argc, char *argv[], firstchar = ReadCommand(&input_message); /* - * (4) disable async signal conditions again. + * (4) turn off the idle-in-transaction and idle-session timeouts, if + * active. We do this before step (5) so that any last-moment timeout + * is certain to be detected in step (5). * - * Query cancel is supposed to be a no-op when there is no query in - * progress, so if a query cancel arrived while we were idle, just - * reset QueryCancelPending. ProcessInterrupts() has that effect when - * it's called when DoingCommandRead is set, so check for interrupts - * before resetting DoingCommandRead. - */ - CHECK_FOR_INTERRUPTS(); - DoingCommandRead = false; - - /* - * (5) turn off the idle-in-transaction and idle-session timeouts, if - * active. - * - * At most one of these two will be active, so there's no need to + * At most one of these timeouts will be active, so there's no need to * worry about combining the timeout.c calls into one. */ if (idle_in_transaction_timeout_enabled) @@ -4352,6 +4341,18 @@ PostgresMain(int argc, char *argv[], idle_session_timeout_enabled = false; } + /* + * (5) disable async signal conditions again. + * + * Query cancel is supposed to be a no-op when there is no query in + * progress, so if a query cancel arrived while we were idle, just + * reset QueryCancelPending. ProcessInterrupts() has that effect when + * it's called when DoingCommandRead is set, so check for interrupts + * before resetting DoingCommandRead. + */ + CHECK_FOR_INTERRUPTS(); + DoingCommandRead = false; + /* * (6) check for any other interesting events that happened while we * slept.