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

Commit 9343bfe

Browse files
committed
Fix memory leak when guc.c decides a setting can't be applied now.
The prohibitValueChange code paths in set_config_option(), which are executed whenever we re-read a PGC_POSTMASTER variable from postgresql.conf, neglected to free anything before exiting. Thus we'd leak the proposed new value of a PGC_STRING variable, as noted by BoChen in bug #16666. For all variable types, if the check hook creates an "extra" chunk, we'd also leak that. These are malloc not palloc chunks, so there is no mechanism for recovering the leaks before process exit. Fortunately, the values are typically not very large, meaning you'd have to go through an awful lot of SIGHUP configuration-reload cycles to make the leakage amount to anything. Still, for a long-lived postmaster process it could potentially be a problem. Oversight in commit 2594cf0. Back-patch to all supported branches. Discussion: https://postgr.es/m/16666-2c41a4eec61b03e1@postgresql.org
1 parent 5efa788 commit 9343bfe

File tree

1 file changed

+30
-2
lines changed
  • src/backend/utils/misc

1 file changed

+30
-2
lines changed

src/backend/utils/misc/guc.c

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7204,6 +7204,10 @@ set_config_option(const char *name, const char *value,
72047204

72057205
if (prohibitValueChange)
72067206
{
7207+
/* Release newextra, unless it's reset_extra */
7208+
if (newextra && !extra_field_used(&conf->gen, newextra))
7209+
free(newextra);
7210+
72077211
if (*conf->variable != newval)
72087212
{
72097213
record->status |= GUC_PENDING_RESTART;
@@ -7294,6 +7298,10 @@ set_config_option(const char *name, const char *value,
72947298

72957299
if (prohibitValueChange)
72967300
{
7301+
/* Release newextra, unless it's reset_extra */
7302+
if (newextra && !extra_field_used(&conf->gen, newextra))
7303+
free(newextra);
7304+
72977305
if (*conf->variable != newval)
72987306
{
72997307
record->status |= GUC_PENDING_RESTART;
@@ -7384,6 +7392,10 @@ set_config_option(const char *name, const char *value,
73847392

73857393
if (prohibitValueChange)
73867394
{
7395+
/* Release newextra, unless it's reset_extra */
7396+
if (newextra && !extra_field_used(&conf->gen, newextra))
7397+
free(newextra);
7398+
73877399
if (*conf->variable != newval)
73887400
{
73897401
record->status |= GUC_PENDING_RESTART;
@@ -7490,9 +7502,21 @@ set_config_option(const char *name, const char *value,
74907502

74917503
if (prohibitValueChange)
74927504
{
7505+
bool newval_different;
7506+
74937507
/* newval shouldn't be NULL, so we're a bit sloppy here */
7494-
if (*conf->variable == NULL || newval == NULL ||
7495-
strcmp(*conf->variable, newval) != 0)
7508+
newval_different = (*conf->variable == NULL ||
7509+
newval == NULL ||
7510+
strcmp(*conf->variable, newval) != 0);
7511+
7512+
/* Release newval, unless it's reset_val */
7513+
if (newval && !string_field_used(conf, newval))
7514+
free(newval);
7515+
/* Release newextra, unless it's reset_extra */
7516+
if (newextra && !extra_field_used(&conf->gen, newextra))
7517+
free(newextra);
7518+
7519+
if (newval_different)
74967520
{
74977521
record->status |= GUC_PENDING_RESTART;
74987522
ereport(elevel,
@@ -7587,6 +7611,10 @@ set_config_option(const char *name, const char *value,
75877611

75887612
if (prohibitValueChange)
75897613
{
7614+
/* Release newextra, unless it's reset_extra */
7615+
if (newextra && !extra_field_used(&conf->gen, newextra))
7616+
free(newextra);
7617+
75907618
if (*conf->variable != newval)
75917619
{
75927620
record->status |= GUC_PENDING_RESTART;

0 commit comments

Comments
 (0)