NotificationsYou must be signed in to change notification settings
Fork6
Star31

Commit6b51fe8

committed

Be more rigorous about local variables in PostgresMain().

Since PostgresMain calls sigsetjmp, any local variables that are notmarked "volatile" have a risk of unspecified behavior. In practicethis means that when control returns via longjmp, such variables mightget reset to their values as of the time of sigsetjmp, depending onwhether the compiler chose to put them in registers or on the stack.We were careful about this for "send_ready_for_query", but not theother local variables.In the case of the timeout_enabled flags, resetting them totheir initial "false" states is actually good, since we do"disable_all_timeouts()" in the longjmp cleanup code path. If thatdoes not happen, we risk uselessly calling "disable_timeout()" later,which is harmless but a little bit expensive. Let's explicitly resetthese 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 changein this logic could re-introduce a problem.)There is no issue for "firstchar" and "input_message" because thoseare explicitly reinitialized each time through the query processingloop. To make that clearer, move them to be declared inside the loop.That leaves us with all the function-lifespan locals except thesigjmp_buf itself marked as volatile, which seems like a good policyto have going forward.Because of the possibility of extra disable_timeout() calls, thisseems worth back-patching.Sergey Shinderuk and Tom LaneDiscussion:https://postgr.es/m/2eda015b-7dff-47fd-d5e2-f1a9899b90a6@postgrespro.ru

1 parentf5b075a commit6b51fe8Copy full SHA for 6b51fe8

File tree

1 file changed

-5

lines changed

src/backend/tcop
- postgres.c

1 file changed

-5

lines changed

`‎src/backend/tcop/postgres.c`

Lines changed: 9 additions & 5 deletions

Original file line number	Diff line number	Diff line change
`@@ -3823,11 +3823,11 @@ PostgresMain(int argc, char *argv[],`
`3823`	`3823`	`constchar*dbname,`
`3824`	`3824`	`constchar*username)`
`3825`	`3825`	`{`
`3826`		`-intfirstchar;`
`3827`		`-StringInfoDatainput_message;`
`3828`	`3826`	`sigjmp_buflocal_sigjmp_buf;`
	`3827`	`+`
	`3828`	`+/* these must be volatile to ensure state is preserved across longjmp: */`
`3829`	`3829`	`volatileboolsend_ready_for_query= true;`
`3830`		`-booldisable_idle_in_transaction_timeout= false;`
	`3830`	`+volatilebooldisable_idle_in_transaction_timeout= false;`
`3831`	`3831`
`3832`	`3832`	`/* Initialize startup process environment if necessary. */`
`3833`	`3833`	`if (!IsUnderPostmaster)`
`@@ -4111,8 +4111,9 @@ PostgresMain(int argc, char *argv[],`
`4111`	`4111`	`* query cancels from being misreported as timeouts in case we're`
`4112`	`4112`	`* forgetting a timeout cancel.`
`4113`	`4113`	`*/`
`4114`		`-disable_all_timeouts(false);`
`4115`		`-QueryCancelPending= false;/* second to avoid race condition */`
	`4114`	`+disable_all_timeouts(false);/* do first to avoid race condition */`
	`4115`	`+QueryCancelPending= false;`
	`4116`	`+disable_idle_in_transaction_timeout= false;`
`4116`	`4117`
`4117`	`4118`	`/* Not reading from the client anymore. */`
`4118`	`4119`	`DoingCommandRead= false;`
`@@ -4201,6 +4202,9 @@ PostgresMain(int argc, char *argv[],`
`4201`	`4202`
`4202`	`4203`	`for (;;)`
`4203`	`4204`	`{`
	`4205`	`+intfirstchar;`
	`4206`	`+StringInfoDatainput_message;`
	`4207`	`+`
`4204`	`4208`	`/*`
`4205`	`4209`	`* At top of loop, reset extended-query-message flag, so that any`
`4206`	`4210`	`* errors encountered in "idle" state don't provoke skip.`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit6b51fe8

File tree

1 file changed

1 file changed

`‎src/backend/tcop/postgres.c`

0 commit comments