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

Commit f35c117

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 8b231d9 commit f35c117

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
@@ -6973,6 +6973,10 @@ set_config_option(const char *name, const char *value,
69736973

69746974
if (prohibitValueChange)
69756975
{
6976+
/* Release newextra, unless it's reset_extra */
6977+
if (newextra && !extra_field_used(&conf->gen, newextra))
6978+
free(newextra);
6979+
69766980
if (*conf->variable != newval)
69776981
{
69786982
record->status |= GUC_PENDING_RESTART;
@@ -7063,6 +7067,10 @@ set_config_option(const char *name, const char *value,
70637067

70647068
if (prohibitValueChange)
70657069
{
7070+
/* Release newextra, unless it's reset_extra */
7071+
if (newextra && !extra_field_used(&conf->gen, newextra))
7072+
free(newextra);
7073+
70667074
if (*conf->variable != newval)
70677075
{
70687076
record->status |= GUC_PENDING_RESTART;
@@ -7153,6 +7161,10 @@ set_config_option(const char *name, const char *value,
71537161

71547162
if (prohibitValueChange)
71557163
{
7164+
/* Release newextra, unless it's reset_extra */
7165+
if (newextra && !extra_field_used(&conf->gen, newextra))
7166+
free(newextra);
7167+
71567168
if (*conf->variable != newval)
71577169
{
71587170
record->status |= GUC_PENDING_RESTART;
@@ -7259,9 +7271,21 @@ set_config_option(const char *name, const char *value,
72597271

72607272
if (prohibitValueChange)
72617273
{
7274+
bool newval_different;
7275+
72627276
/* newval shouldn't be NULL, so we're a bit sloppy here */
7263-
if (*conf->variable == NULL || newval == NULL ||
7264-
strcmp(*conf->variable, newval) != 0)
7277+
newval_different = (*conf->variable == NULL ||
7278+
newval == NULL ||
7279+
strcmp(*conf->variable, newval) != 0);
7280+
7281+
/* Release newval, unless it's reset_val */
7282+
if (newval && !string_field_used(conf, newval))
7283+
free(newval);
7284+
/* Release newextra, unless it's reset_extra */
7285+
if (newextra && !extra_field_used(&conf->gen, newextra))
7286+
free(newextra);
7287+
7288+
if (newval_different)
72657289
{
72667290
record->status |= GUC_PENDING_RESTART;
72677291
ereport(elevel,
@@ -7356,6 +7380,10 @@ set_config_option(const char *name, const char *value,
73567380

73577381
if (prohibitValueChange)
73587382
{
7383+
/* Release newextra, unless it's reset_extra */
7384+
if (newextra && !extra_field_used(&conf->gen, newextra))
7385+
free(newextra);
7386+
73597387
if (*conf->variable != newval)
73607388
{
73617389
record->status |= GUC_PENDING_RESTART;

0 commit comments

Comments
 (0)