Commit b8d0cda5 authored by Tom Lane's avatar Tom Lane

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.
parent ebb5457c
...@@ -4323,22 +4323,11 @@ PostgresMain(int argc, char *argv[], ...@@ -4323,22 +4323,11 @@ PostgresMain(int argc, char *argv[],
firstchar = ReadCommand(&input_message); 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 * At most one of these timeouts will be active, so there's no need to
* 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
* worry about combining the timeout.c calls into one. * worry about combining the timeout.c calls into one.
*/ */
if (idle_in_transaction_timeout_enabled) if (idle_in_transaction_timeout_enabled)
...@@ -4352,6 +4341,18 @@ PostgresMain(int argc, char *argv[], ...@@ -4352,6 +4341,18 @@ PostgresMain(int argc, char *argv[],
idle_session_timeout_enabled = false; 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 * (6) check for any other interesting events that happened while we
* slept. * slept.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment