From 57ef2da434e235f6295bf49a065fce3da398fd8b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 19 Mar 2018 23:59:17 -0400 Subject: [PATCH] Prevent query-lifespan memory leakage of SP-GiST traversal values. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original coding of the SP-GiST scan traversalValue feature (commit ccd6eb49a) arranged for traversal values to be stored in the query's main executor context. That's fine if there's only one index scan per query, but if there are many, we have a memory leak as successive scans create new traversal values. Fix it by creating a separate memory context for traversal values, which we can reset during spgrescan(). Back-patch to 9.6 where this code was introduced. In principle, adding the traversalCxt field to SpGistScanOpaqueData creates an ABI break in the back branches. But I (tgl) have little sympathy for extensions including spgist_private.h, so I'm not very worried about that. Alternatively we could stick the new field at the end of the struct in back branches, but that has its own downsides. Anton Dignös, reviewed by Alexander Kuzmenkov Discussion: https://postgr.es/m/CALNdv1jb6y2Te-m8xHLxLX12RsBmZJ1f4hESX7J0HjgyOhA9eA@mail.gmail.com --- src/backend/access/spgist/spgscan.c | 9 ++++++++- src/include/access/spgist_private.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/backend/access/spgist/spgscan.c b/src/backend/access/spgist/spgscan.c index 307c6a4ab57..e4d21a402fb 100644 --- a/src/backend/access/spgist/spgscan.c +++ b/src/backend/access/spgist/spgscan.c @@ -194,6 +194,9 @@ spgbeginscan(Relation rel, int keysz, int orderbysz) so->tempCxt = AllocSetContextCreate(CurrentMemoryContext, "SP-GiST search temporary context", ALLOCSET_DEFAULT_SIZES); + so->traversalCxt = AllocSetContextCreate(CurrentMemoryContext, + "SP-GiST traversal-value context", + ALLOCSET_DEFAULT_SIZES); /* Set up indexTupDesc and xs_itupdesc in case it's an index-only scan */ so->indexTupDesc = scan->xs_itupdesc = RelationGetDescr(rel); @@ -209,6 +212,9 @@ spgrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys, { SpGistScanOpaque so = (SpGistScanOpaque) scan->opaque; + /* clear traversal context before proceeding to the next scan */ + MemoryContextReset(so->traversalCxt); + /* copy scankeys into local storage */ if (scankey && scan->numberOfKeys > 0) { @@ -229,6 +235,7 @@ spgendscan(IndexScanDesc scan) SpGistScanOpaque so = (SpGistScanOpaque) scan->opaque; MemoryContextDelete(so->tempCxt); + MemoryContextDelete(so->traversalCxt); } /* @@ -463,7 +470,7 @@ redirect: in.scankeys = so->keyData; in.nkeys = so->numberOfKeys; in.reconstructedValue = stackEntry->reconstructedValue; - in.traversalMemoryContext = oldCtx; + in.traversalMemoryContext = so->traversalCxt; in.traversalValue = stackEntry->traversalValue; in.level = stackEntry->level; in.returnData = so->want_itup; diff --git a/src/include/access/spgist_private.h b/src/include/access/spgist_private.h index cb8fa9c0b51..e788a7761f4 100644 --- a/src/include/access/spgist_private.h +++ b/src/include/access/spgist_private.h @@ -136,6 +136,7 @@ typedef struct SpGistScanOpaqueData { SpGistState state; /* see above */ MemoryContext tempCxt; /* short-lived memory context */ + MemoryContext traversalCxt; /* memory context for traversalValues */ /* Control flags showing whether to search nulls and/or non-nulls */ bool searchNulls; /* scan matches (all) null entries */