Re: make MaxBackends available in _PG_init
Nathan Bossart <bossartn@amazon.com>
From: "Bossart, Nathan" <bossartn@amazon.com>
To: Fujii Masao <masao.fujii@oss.nttdata.com>, "wangsh.fnst@fujitsu.com" <wangsh.fnst@fujitsu.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Andres Freund <andres@anarazel.de>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, "Greg Sabino
Mullane" <htamfids@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2022-01-07T18:09:23Z
Lists: pgsql-hackers
Attachments
- v5-0001-Disallow-external-access-to-MaxBackends.patch (application/octet-stream) patch v5-0001
On 1/7/22, 8:54 AM, "Fujii Masao" <masao.fujii@oss.nttdata.com> wrote: > The patch handles only MaxBackends. But isn't there other variable having the same issue? I wouldn't be surprised to learn of other cases, but I've only encountered this specific issue with MaxBackends. I think MaxBackends is notable because it is more likely to be used by preloaded libraries but is intentionally initialized after loading them. As noted in an earlier message on this thread [0], using MaxBackends in a call to RequestAddinShmemSpace() in _PG_init() may not reliably cause problems, too. > It seems overkill to remove "extern" from MaxBackends and replace MaxBackends with GetMaxBackends() in the existing PostgreSQL codes. I'm not sure how much it's actually worth doing that. Instead, isn't it enough to just add the comment like "Use GetMaxBackends() if you want to treat the lookup for uninitialized MaxBackends as an error" in the definition of MaxBackends? While that approach would provide a way to safely retrieve the value, I think it would do little to prevent the issue in practice. If the size of the patch is a concern, we could also convert MaxBackends into a macro for calling GetMaxBackends(). This could also be a nice way to avoid breaking extensions that are using it correctly while triggering ERRORs for extensions that are using it incorrectly. I've attached a new version of the patch that does it this way. Nathan [0] https://postgr.es/m/8499D41B-628A-4CE0-9139-00D718F9D06B%40amazon.com
Commits
-
Add a new shmem_request_hook hook.
- 4f2400cb3f10 15.0 landed
-
Remove non-functional code for unloading loadable modules.
- ab02d702ef08 15.0 landed
-
Fix misleading comments about background worker registration.
- 701d918a426b 15.0 landed
-
Revert the addition of GetMaxBackends() and related stuff.
- 7fc0e7de9fb8 15.0 landed
-
Fix typo in multixact.c
- 0147fc7c8c92 15.0 landed
-
Reduce more the number of calls to GetMaxBackends()
- 4567596316d1 15.0 landed
-
Remove MaxBackends variable in favor of GetMaxBackends() function.
- aa64f23b0292 15.0 landed
-
Fix comments about bgworker registration before MaxBackends initialization
- 5ecd0183fb6a 15.0 landed
-
Fix bogus assertion in BootstrapModeMain().
- e12694523e7e 15.0 cited
-
Make sure MaxBackends is always set
- dfbba2c86cc8 9.3.0 cited