Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Further second thoughts about idle_session_timeout patch.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 7 Jan 2021 16:45:09 +0000 (11:45 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 7 Jan 2021 16:45:09 +0000 (11:45 -0500)
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

index 899723470d66b112e636562badab749e61389579..174c72a14bc4b70c0229a80003f398a889c6caed 100644 (file)
@@ -4269,7 +4269,18 @@ PostgresMain(int argc, char *argv[],
        firstchar = ReadCommand(&input_message);
 
        /*
-        * (4) disable async signal conditions again.
+        * (4) turn off the idle-in-transaction timeout, if active.  We do
+        * this before step (5) so that any last-moment timeout is certain to
+        * be detected in step (5).
+        */
+       if (disable_idle_in_transaction_timeout)
+       {
+           disable_timeout(IDLE_IN_TRANSACTION_SESSION_TIMEOUT, false);
+           disable_idle_in_transaction_timeout = 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
@@ -4280,15 +4291,6 @@ PostgresMain(int argc, char *argv[],
        CHECK_FOR_INTERRUPTS();
        DoingCommandRead = false;
 
-       /*
-        * (5) turn off the idle-in-transaction timeout
-        */
-       if (disable_idle_in_transaction_timeout)
-       {
-           disable_timeout(IDLE_IN_TRANSACTION_SESSION_TIMEOUT, false);
-           disable_idle_in_transaction_timeout = false;
-       }
-
        /*
         * (6) check for any other interesting events that happened while we
         * slept.