Prevent memory leaks in parseRelOptions().
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 13 Aug 2014 15:35:51 +0000 (11:35 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 13 Aug 2014 15:35:51 +0000 (11:35 -0400)
parseRelOptions() tended to leak memory in the caller's context.  Most
of the time this doesn't really matter since the caller's context is
at most query-lifespan, and the function won't be invoked very many times.
However, when testing with CLOBBER_CACHE_RECURSIVELY, the same relcache
entry can get rebuilt a *lot* of times in one query, leading to significant
intraquery memory bloat if it has any reloptions.  Noted while
investigating a related report from Tomas Vondra.

In passing, get rid of some Asserts that are redundant with the one
done by deconstruct_array().

As with other patches to avoid leaks in CLOBBER_CACHE testing, it doesn't
really seem worth back-patching this.

src/backend/access/common/reloptions.c

index c7ad6f96f863d618235f2abc298cccea384a3508..e0b81b9eb5139e8db3c4de25ca3bec7ffaab21bb 100644 (file)
@@ -620,8 +620,6 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
        int         noldoptions;
        int         i;
 
-       Assert(ARR_ELEMTYPE(array) == TEXTOID);
-
        deconstruct_array(array, TEXTOID, -1, false, 'i',
                          &oldoptions, NULL, &noldoptions);
 
@@ -777,8 +775,6 @@ untransformRelOptions(Datum options)
 
    array = DatumGetArrayTypeP(options);
 
-   Assert(ARR_ELEMTYPE(array) == TEXTOID);
-
    deconstruct_array(array, TEXTOID, -1, false, 'i',
                      &optiondatums, NULL, &noptions);
 
@@ -912,14 +908,10 @@ parseRelOptions(Datum options, bool validate, relopt_kind kind,
    /* Done if no options */
    if (PointerIsValid(DatumGetPointer(options)))
    {
-       ArrayType  *array;
+       ArrayType  *array = DatumGetArrayTypeP(options);
        Datum      *optiondatums;
        int         noptions;
 
-       array = DatumGetArrayTypeP(options);
-
-       Assert(ARR_ELEMTYPE(array) == TEXTOID);
-
        deconstruct_array(array, TEXTOID, -1, false, 'i',
                          &optiondatums, NULL, &noptions);
 
@@ -959,6 +951,11 @@ parseRelOptions(Datum options, bool validate, relopt_kind kind,
                         errmsg("unrecognized parameter \"%s\"", s)));
            }
        }
+
+       /* It's worth avoiding memory leaks in this function */
+       pfree(optiondatums);
+       if (((void *) array) != DatumGetPointer(options))
+           pfree(array);
    }
 
    *numrelopts = numoptions;