Skip to content

Navigation Menu

Sign in
Appearance settings

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

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit b35617d

Browse filesBrowse files
tglsfdctglsfdc
committed
Process session_preload_libraries within InitPostgres's transaction.
Previously we did this after InitPostgres, at a somewhat randomly chosen place within PostgresMain. However, since commit a0ffa88 doing this outside a transaction can cause a crash, if we need to check permissions while replacing a placeholder GUC. (Besides which, a preloaded library could itself want to do database access within _PG_init.) To avoid needing an additional transaction start/end in every session, move the process_session_preload_libraries call to within InitPostgres's transaction. That requires teaching the code not to call it when InitPostgres is called from somewhere other than PostgresMain, since we don't want session_preload_libraries to affect background workers. The most future-proof solution here seems to be to add an additional flag parameter to InitPostgres; fortunately, we're not yet very worried about API stability for v15. Doing this also exposed the fact that we're currently honoring session_preload_libraries in walsenders, even those not connected to any database. This seems, at minimum, a POLA violation: walsenders are not interactive sessions. Let's stop doing that. (All these comments also apply to local_preload_libraries, of course.) Per report from Gurjeet Singh (thanks also to Nathan Bossart and Kyotaro Horiguchi for review). Backpatch to v15 where a0ffa88 came in. Discussion: https://postgr.es/m/CABwTF4VEpwTHhRQ+q5MiC5ucngN-whN-PdcKeufX7eLSoAfbZA@mail.gmail.com
1 parent 7a08f78 commit b35617d
Copy full SHA for b35617d

File tree

6 files changed

+64
-25
lines changed
Filter options

6 files changed

+64
-25
lines changed

‎src/backend/bootstrap/bootstrap.c

Copy file name to clipboardExpand all lines: src/backend/bootstrap/bootstrap.c
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
354354
if (pg_link_canary_is_frontend())
355355
elog(ERROR, "backend is incorrectly linked to frontend functions");
356356

357-
InitPostgres(NULL, InvalidOid, NULL, InvalidOid, NULL, false);
357+
InitPostgres(NULL, InvalidOid, NULL, InvalidOid, false, false, NULL);
358358

359359
/* Initialize stuff for bootstrap-file processing */
360360
for (i = 0; i < MAXATTR; i++)

‎src/backend/postmaster/autovacuum.c

Copy file name to clipboardExpand all lines: src/backend/postmaster/autovacuum.c
+4-3Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ AutoVacLauncherMain(int argc, char *argv[])
475475
/* Early initialization */
476476
BaseInit();
477477

478-
InitPostgres(NULL, InvalidOid, NULL, InvalidOid, NULL, false);
478+
InitPostgres(NULL, InvalidOid, NULL, InvalidOid, false, false, NULL);
479479

480480
SetProcessingMode(NormalProcessing);
481481

@@ -1694,12 +1694,13 @@ AutoVacWorkerMain(int argc, char *argv[])
16941694
pgstat_report_autovac(dbid);
16951695

16961696
/*
1697-
* Connect to the selected database
1697+
* Connect to the selected database, specifying no particular user
16981698
*
16991699
* Note: if we have selected a just-deleted database (due to using
17001700
* stale stats info), we'll fail and exit here.
17011701
*/
1702-
InitPostgres(NULL, dbid, NULL, InvalidOid, dbname, false);
1702+
InitPostgres(NULL, dbid, NULL, InvalidOid, false, false,
1703+
dbname);
17031704
SetProcessingMode(NormalProcessing);
17041705
set_ps_display(dbname);
17051706
ereport(DEBUG1,

‎src/backend/postmaster/postmaster.c

Copy file name to clipboardExpand all lines: src/backend/postmaster/postmaster.c
+10-2Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5654,7 +5654,11 @@ BackgroundWorkerInitializeConnection(const char *dbname, const char *username, u
56545654
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
56555655
errmsg("database connection requirement not indicated during registration")));
56565656

5657-
InitPostgres(dbname, InvalidOid, username, InvalidOid, NULL, (flags & BGWORKER_BYPASS_ALLOWCONN) != 0);
5657+
InitPostgres(dbname, InvalidOid, /* database to connect to */
5658+
username, InvalidOid, /* role to connect as */
5659+
false, /* never honor session_preload_libraries */
5660+
(flags & BGWORKER_BYPASS_ALLOWCONN) != 0, /* ignore datallowconn? */
5661+
NULL); /* no out_dbname */
56585662

56595663
/* it had better not gotten out of "init" mode yet */
56605664
if (!IsInitProcessingMode())
@@ -5677,7 +5681,11 @@ BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, uint32 flags)
56775681
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
56785682
errmsg("database connection requirement not indicated during registration")));
56795683

5680-
InitPostgres(NULL, dboid, NULL, useroid, NULL, (flags & BGWORKER_BYPASS_ALLOWCONN) != 0);
5684+
InitPostgres(NULL, dboid, /* database to connect to */
5685+
NULL, useroid, /* role to connect as */
5686+
false, /* never honor session_preload_libraries */
5687+
(flags & BGWORKER_BYPASS_ALLOWCONN) != 0, /* ignore datallowconn? */
5688+
NULL); /* no out_dbname */
56815689

56825690
/* it had better not gotten out of "init" mode yet */
56835691
if (!IsInitProcessingMode())

‎src/backend/tcop/postgres.c

Copy file name to clipboardExpand all lines: src/backend/tcop/postgres.c
+5-7Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4076,7 +4076,11 @@ PostgresMain(const char *dbname, const char *username)
40764076
* it inside InitPostgres() instead. In particular, anything that
40774077
* involves database access should be there, not here.
40784078
*/
4079-
InitPostgres(dbname, InvalidOid, username, InvalidOid, NULL, false);
4079+
InitPostgres(dbname, InvalidOid, /* database to connect to */
4080+
username, InvalidOid, /* role to connect as */
4081+
!am_walsender, /* honor session_preload_libraries? */
4082+
false, /* don't ignore datallowconn */
4083+
NULL); /* no out_dbname */
40804084

40814085
/*
40824086
* If the PostmasterContext is still around, recycle the space; we don't
@@ -4112,12 +4116,6 @@ PostgresMain(const char *dbname, const char *username)
41124116
if (am_walsender)
41134117
InitWalSender();
41144118

4115-
/*
4116-
* process any libraries that should be preloaded at backend start (this
4117-
* likewise can't be done until GUC settings are complete)
4118-
*/
4119-
process_session_preload_libraries();
4120-
41214119
/*
41224120
* Send this backend's cancellation info to the frontend.
41234121
*/

‎src/backend/utils/init/postinit.c

Copy file name to clipboardExpand all lines: src/backend/utils/init/postinit.c
+39-10Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -622,29 +622,48 @@ BaseInit(void)
622622
* InitPostgres
623623
* Initialize POSTGRES.
624624
*
625+
* Parameters:
626+
* in_dbname, dboid: specify database to connect to, as described below
627+
* username, useroid: specify role to connect as, as described below
628+
* load_session_libraries: TRUE to honor [session|local]_preload_libraries
629+
* override_allow_connections: TRUE to connect despite !datallowconn
630+
* out_dbname: optional output parameter, see below; pass NULL if not used
631+
*
625632
* The database can be specified by name, using the in_dbname parameter, or by
626-
* OID, using the dboid parameter. In the latter case, the actual database
633+
* OID, using the dboid parameter. Specify NULL or InvalidOid respectively
634+
* for the unused parameter. If dboid is provided, the actual database
627635
* name can be returned to the caller in out_dbname. If out_dbname isn't
628636
* NULL, it must point to a buffer of size NAMEDATALEN.
629637
*
630-
* Similarly, the username can be passed by name, using the username parameter,
638+
* Similarly, the role can be passed by name, using the username parameter,
631639
* or by OID using the useroid parameter.
632640
*
633-
* In bootstrap mode no parameters are used. The autovacuum launcher process
634-
* doesn't use any parameters either, because it only goes far enough to be
635-
* able to read pg_database; it doesn't connect to any particular database.
636-
* In walsender mode only username is used.
641+
* In bootstrap mode the database and username parameters are NULL/InvalidOid.
642+
* The autovacuum launcher process doesn't specify these parameters either,
643+
* because it only goes far enough to be able to read pg_database; it doesn't
644+
* connect to any particular database. An autovacuum worker specifies a
645+
* database but not a username; conversely, a physical walsender specifies
646+
* username but not database.
647+
*
648+
* By convention, load_session_libraries should be passed as true in
649+
* "interactive" sessions (including standalone backends), but false in
650+
* background processes such as autovacuum. Note in particular that it
651+
* shouldn't be true in parallel worker processes; those have another
652+
* mechanism for replicating their leader's set of loaded libraries.
637653
*
638-
* As of PostgreSQL 8.2, we expect InitProcess() was already called, so we
639-
* already have a PGPROC struct ... but it's not completely filled in yet.
654+
* We expect that InitProcess() was already called, so we already have a
655+
* PGPROC struct ... but it's not completely filled in yet.
640656
*
641657
* Note:
642658
* Be very careful with the order of calls in the InitPostgres function.
643659
* --------------------------------
644660
*/
645661
void
646-
InitPostgres(const char *in_dbname, Oid dboid, const char *username,
647-
Oid useroid, char *out_dbname, bool override_allow_connections)
662+
InitPostgres(const char *in_dbname, Oid dboid,
663+
const char *username, Oid useroid,
664+
bool load_session_libraries,
665+
bool override_allow_connections,
666+
char *out_dbname)
648667
{
649668
bool bootstrap = IsBootstrapProcessingMode();
650669
bool am_superuser;
@@ -1108,6 +1127,16 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
11081127
/* Initialize this backend's session state. */
11091128
InitializeSession();
11101129

1130+
/*
1131+
* If this is an interactive session, load any libraries that should be
1132+
* preloaded at backend start. Since those are determined by GUCs, this
1133+
* can't happen until GUC settings are complete, but we want it to happen
1134+
* during the initial transaction in case anything that requires database
1135+
* access needs to be done.
1136+
*/
1137+
if (load_session_libraries)
1138+
process_session_preload_libraries();
1139+
11111140
/* report this backend in the PgBackendStatus array */
11121141
if (!bootstrap)
11131142
pgstat_bestart();

‎src/include/miscadmin.h

Copy file name to clipboardExpand all lines: src/include/miscadmin.h
+5-2Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -449,8 +449,11 @@ extern PGDLLIMPORT AuxProcType MyAuxProcType;
449449
/* in utils/init/postinit.c */
450450
extern void pg_split_opts(char **argv, int *argcp, const char *optstr);
451451
extern void InitializeMaxBackends(void);
452-
extern void InitPostgres(const char *in_dbname, Oid dboid, const char *username,
453-
Oid useroid, char *out_dbname, bool override_allow_connections);
452+
extern void InitPostgres(const char *in_dbname, Oid dboid,
453+
const char *username, Oid useroid,
454+
bool load_session_libraries,
455+
bool override_allow_connections,
456+
char *out_dbname);
454457
extern void BaseInit(void);
455458

456459
/* in utils/init/miscinit.c */

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.