Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit a8b7424

Browse files
committed
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
1 parent a44d96a commit a8b7424

File tree

1 file changed

+11
-6
lines changed

1 file changed

+11
-6
lines changed

src/backend/tcop/postgres.c

+11-6
Original file line numberDiff line numberDiff line change
@@ -4111,12 +4111,12 @@ PostgresSingleUserMain(int argc, char *argv[],
41114111
void
41124112
PostgresMain(const char *dbname, const char *username)
41134113
{
4114-
int firstchar;
4115-
StringInfoData input_message;
41164114
sigjmp_buf local_sigjmp_buf;
4115+
4116+
/* these must be volatile to ensure state is preserved across longjmp: */
41174117
volatile bool send_ready_for_query = true;
4118-
bool idle_in_transaction_timeout_enabled = false;
4119-
bool idle_session_timeout_enabled = false;
4118+
volatile bool idle_in_transaction_timeout_enabled = false;
4119+
volatile bool idle_session_timeout_enabled = false;
41204120

41214121
Assert(dbname != NULL);
41224122
Assert(username != NULL);
@@ -4322,8 +4322,10 @@ PostgresMain(const char *dbname, const char *username)
43224322
* query cancels from being misreported as timeouts in case we're
43234323
* forgetting a timeout cancel.
43244324
*/
4325-
disable_all_timeouts(false);
4326-
QueryCancelPending = false; /* second to avoid race condition */
4325+
disable_all_timeouts(false); /* do first to avoid race condition */
4326+
QueryCancelPending = false;
4327+
idle_in_transaction_timeout_enabled = false;
4328+
idle_session_timeout_enabled = false;
43274329

43284330
/* Not reading from the client anymore. */
43294331
DoingCommandRead = false;
@@ -4418,6 +4420,9 @@ PostgresMain(const char *dbname, const char *username)
44184420

44194421
for (;;)
44204422
{
4423+
int firstchar;
4424+
StringInfoData input_message;
4425+
44214426
/*
44224427
* At top of loop, reset extended-query-message flag, so that any
44234428
* errors encountered in "idle" state don't provoke skip.

0 commit comments

Comments
 (0)