postgres.git / summary / log / commit / refs
commit 93dcdfa88f5f9befb781558920e02d3ee86dc629
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon Jul 10 16:14:34 2023 +0000
First released in: 17.0, 11.21, 12.16, 13.12, 14.9, 15.4, 16.0
Be more rigorous about local variables in PostgresMain().
Since PostgresMain calls sigsetjmp, any local variables that are not
marked "volatile" have a risk of unspecified behavior. In practice
this means that when control returns via longjmp, such variables might
get reset to their values as of the time of sigsetjmp, depending on
whether the compiler chose to put them in registers or on the stack.
We were careful about this for "send_ready_for_query", but not the
other local variables.
In the case of the timeout_enabled flags, resetting them to
their initial "false" states is actually good, since we do
"disable_all_timeouts()" in the longjmp cleanup code path. If that
does not happen, we risk uselessly calling "disable_timeout()" later,
which is harmless but a little bit expensive. Let's explicitly reset
these flags so that the behavior is correct and platform-independent.
(This change means that we really don't need the new "volatile"
markings after all, but let's install them anyway since any change
in this logic could re-introduce a problem.)
There is no issue for "firstchar" and "input_message" because those
are explicitly reinitialized each time through the query processing
loop. To make that clearer, move them to be declared inside the loop.
That leaves us with all the function-lifespan locals except the
sigjmp_buf itself marked as volatile, which seems like a good policy
to have going forward.
Because of the possibility of extra disable_timeout() calls, this
seems worth back-patching.
Sergey Shinderuk and Tom Lane
Discussion: https://postgr.es/m/2eda015b-7dff-47fd-d5e2-f1a9899b90a6@postgrespro.ru
src/backend/tcop/postgres.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index b46b0551f94..36cc99ec9cf 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4111,12 +4111,12 @@ PostgresSingleUserMain(int argc, char *argv[],
void
PostgresMain(const char *dbname, const char *username)
{
- int firstchar;
- StringInfoData input_message;
sigjmp_buf local_sigjmp_buf;
+
+ /* these must be volatile to ensure state is preserved across longjmp: */
volatile bool send_ready_for_query = true;
- bool idle_in_transaction_timeout_enabled = false;
- bool idle_session_timeout_enabled = false;
+ volatile bool idle_in_transaction_timeout_enabled = false;
+ volatile bool idle_session_timeout_enabled = false;
Assert(dbname != NULL);
Assert(username != NULL);
@@ -4322,8 +4322,10 @@ PostgresMain(const char *dbname, const char *username)
* query cancels from being misreported as timeouts in case we're
* forgetting a timeout cancel.
*/
- disable_all_timeouts(false);
- QueryCancelPending = false; /* second to avoid race condition */
+ disable_all_timeouts(false); /* do first to avoid race condition */
+ QueryCancelPending = false;
+ idle_in_transaction_timeout_enabled = false;
+ idle_session_timeout_enabled = false;
/* Not reading from the client anymore. */
DoingCommandRead = false;
@@ -4418,6 +4420,9 @@ PostgresMain(const char *dbname, const char *username)
for (;;)
{
+ int firstchar;
+ StringInfoData input_message;
+
/*
* At top of loop, reset extended-query-message flag, so that any
* errors encountered in "idle" state don't provoke skip.
[parent: e004cb039440]