Fix memory leak in repeated SPGIST index scans.
authorTom Lane
Wed, 31 Oct 2018 21:04:43 +0000 (17:04 -0400)
committerTom Lane
Wed, 31 Oct 2018 21:04:43 +0000 (17:04 -0400)
spgendscan neglected to pfree all the memory allocated by spgbeginscan.
It's possible to get away with that in most normal queries, since the
memory is allocated in the executor's per-query context which is about
to get deleted anyway; but it causes severe memory leakage during
creation or filling of large exclusion-constraint indexes.

Also, document that amendscan is supposed to free what ambeginscan
allocates.  The docs' lack of clarity on that point probably caused this
bug to begin with.  (There is discussion of changing that API spec going
forward, but I don't think it'd be appropriate for the back branches.)

Per report from Bruno Wolff.  It's been like this since the beginning,
so back-patch to all active branches.

In HEAD, also fix an independent leak caused by commit 2a6368343
(allocating memory during spgrescan instead of spgbeginscan, which
might be all right if it got cleaned up, but it didn't).  And do a bit
of code beautification on that commit, too.

Discussion: https://postgr.es/m/20181024012314[email protected]

doc/src/sgml/indexam.sgml
src/backend/access/spgist/spgscan.c

index 8b2999e41c5a7c0f0234028a43cd9a47e3426290..d7a2ebfb67de56e0346cb77b211752222bb48110 100644 (file)
@@ -612,7 +612,8 @@ amendscan (IndexScanDesc scan);
 
    End a scan and release resources.  The scan struct itself
    should not be freed, but any locks or pins taken internally by the
-   access method must be released.
+   access method must be released, as well as any other memory allocated
+   by ambeginscan and other scan-related functions.
   
 
   
index ae0e0a9e7300f183008aae1104d58fe5e27b0dc1..9ae2c8efb616061f490f2a3f90524d3159ba7298 100644 (file)
@@ -240,6 +240,14 @@ spgendscan(IndexScanDesc scan)
 
    MemoryContextDelete(so->tempCxt);
    MemoryContextDelete(so->traversalCxt);
+
+   if (so->keyData)
+       pfree(so->keyData);
+
+   if (so->state.deadTupleStorage)
+       pfree(so->state.deadTupleStorage);
+
+   pfree(so);
 }
 
 /*