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.

1 files changed, 16 insertions, 15 deletions

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 2b53ebf97dc..28055680aad 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)
@@ -4353,6 +4342,18 @@ PostgresMain(int argc, char *argv[],
 		}
 
 		/*
+		 * (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.
 		 */