Fix query-duration memory leak with GIN rescans.

The requiredEntries / additionalEntries arrays were not freed in
freeScanKeys() like other per-key stuff.

It's not obvious, but startScanKey() was only ever called after the keys
have been initialized with ginNewScanKey(). That's why it doesn't need to
worry about freeing existing arrays. The ginIsNewKey() test in gingetbitmap
was never true, because ginrescan free's the existing keys, and it's not OK
to call gingetbitmap twice in a row without calling ginrescan in between.
To make that clear, remove the unnecessary ginIsNewKey(). And just to be
extra sure that nothing funny happens if there is an existing key after all,
call freeScanKeys() to free it if it exists. This makes the code more
straightforward.

(I'm seeing other similar leaks in testing a query that rescans an GIN index
scan, but that's a different issue. This just fixes the obvious leak with
those two arrays.)

Backpatch to 9.4, where GIN fast scan was added.

diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index f78c9fcc9c7f6506448c5dac0a1fc672a72ab4b5..96c1cb971e4c50203b4001a8b3a0f8f1e09ee950 100644 (file)
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -560,6 +560,10 @@ startScan(IndexScanDesc scan)
                }
        }
 
+       /*
+        * Now that we have the estimates for the entry frequencies, finish
+        * initializing the scan keys.
+        */
        for (i = 0; i < so->nkeys; i++)
                startScanKey(ginstate, so, so->keys + i);
 }
@@ -1763,7 +1767,6 @@ scanPendingInsert(IndexScanDesc scan, TIDBitmap *tbm, int64 *ntids)
 }
 
 
-#define GinIsNewKey(s)         ( ((GinScanOpaque) scan->opaque)->keys == NULL )
 #define GinIsVoidRes(s)                ( ((GinScanOpaque) scan->opaque)->isVoidRes )
 
 Datum
@@ -1771,6 +1774,7 @@ gingetbitmap(PG_FUNCTION_ARGS)
 {
        IndexScanDesc scan = (IndexScanDesc) PG_GETARG_POINTER(0);
        TIDBitmap  *tbm = (TIDBitmap *) PG_GETARG_POINTER(1);
+       GinScanOpaque so = (GinScanOpaque) scan->opaque;
        int64           ntids;
        ItemPointerData iptr;
        bool            recheck;
@@ -1778,8 +1782,8 @@ gingetbitmap(PG_FUNCTION_ARGS)
        /*
         * Set up the scan keys, and check for unsatisfiable query.
         */
-       if (GinIsNewKey(scan))
-               ginNewScanKey(scan);
+       ginFreeScanKeys(so); /* there should be no keys yet, but just to be sure */
+       ginNewScanKey(scan);
 
        if (GinIsVoidRes(scan))
                PG_RETURN_INT64(0);

diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index 66c62b2e32a2dcf3c57a07605833615f00f5c51d..0b9c450b89e1134e9e29cb4cdb6d46ba2c5c2a12 100644 (file)
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -163,6 +163,10 @@ ginFillScanKey(GinScanOpaque so, OffsetNumber attnum,
        key->curItemMatches = false;
        key->recheckCurItem = false;
        key->isFinished = false;
+       key->nrequired = 0;
+       key->nadditional = 0;
+       key->requiredEntries = NULL;
+       key->additionalEntries = NULL;
 
        ginInitConsistentFunction(ginstate, key);
 
@@ -223,8 +227,8 @@ ginFillScanKey(GinScanOpaque so, OffsetNumber attnum,
        }
 }
 
-static void
-freeScanKeys(GinScanOpaque so)
+void
+ginFreeScanKeys(GinScanOpaque so)
 {
        uint32          i;
 
@@ -237,6 +241,10 @@ freeScanKeys(GinScanOpaque so)
 
                pfree(key->scanEntry);
                pfree(key->entryRes);
+               if (key->requiredEntries)
+                       pfree(key->requiredEntries);
+               if (key->additionalEntries)
+                       pfree(key->additionalEntries);
        }
 
        pfree(so->keys);
@@ -416,7 +424,7 @@ ginrescan(PG_FUNCTION_ARGS)
        /* remaining arguments are ignored */
        GinScanOpaque so = (GinScanOpaque) scan->opaque;
 
-       freeScanKeys(so);
+       ginFreeScanKeys(so);
 
        if (scankey && scan->numberOfKeys > 0)
        {
@@ -434,7 +442,7 @@ ginendscan(PG_FUNCTION_ARGS)
        IndexScanDesc scan = (IndexScanDesc) PG_GETARG_POINTER(0);
        GinScanOpaque so = (GinScanOpaque) scan->opaque;
 
-       freeScanKeys(so);
+       ginFreeScanKeys(so);
 
        MemoryContextDelete(so->tempCtx);
 

diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index d179319b1e57dba80e252009c9220d12eb342524..000dda11b45e73441677747ac4592af9aeb3bf04 100644 (file)
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -890,6 +890,7 @@ extern Datum ginrescan(PG_FUNCTION_ARGS);
 extern Datum ginmarkpos(PG_FUNCTION_ARGS);
 extern Datum ginrestrpos(PG_FUNCTION_ARGS);
 extern void ginNewScanKey(IndexScanDesc scan);
+extern void ginFreeScanKeys(GinScanOpaque so);
 
 /* ginget.c */
 extern Datum gingetbitmap(PG_FUNCTION_ARGS);