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

Commit dc40ca6

Browse files
committed
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.
1 parent cb01685 commit dc40ca6

File tree

3 files changed

+20
-7
lines changed

3 files changed

+20
-7
lines changed

src/backend/access/gin/ginget.c

+7-3
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,10 @@ startScan(IndexScanDesc scan)
560560
}
561561
}
562562

563+
/*
564+
* Now that we have the estimates for the entry frequencies, finish
565+
* initializing the scan keys.
566+
*/
563567
for (i = 0; i < so->nkeys; i++)
564568
startScanKey(ginstate, so, so->keys + i);
565569
}
@@ -1763,23 +1767,23 @@ scanPendingInsert(IndexScanDesc scan, TIDBitmap *tbm, int64 *ntids)
17631767
}
17641768

17651769

1766-
#define GinIsNewKey(s) ( ((GinScanOpaque) scan->opaque)->keys == NULL )
17671770
#define GinIsVoidRes(s) ( ((GinScanOpaque) scan->opaque)->isVoidRes )
17681771

17691772
Datum
17701773
gingetbitmap(PG_FUNCTION_ARGS)
17711774
{
17721775
IndexScanDesc scan = (IndexScanDesc) PG_GETARG_POINTER(0);
17731776
TIDBitmap *tbm = (TIDBitmap *) PG_GETARG_POINTER(1);
1777+
GinScanOpaque so = (GinScanOpaque) scan->opaque;
17741778
int64 ntids;
17751779
ItemPointerData iptr;
17761780
bool recheck;
17771781

17781782
/*
17791783
* Set up the scan keys, and check for unsatisfiable query.
17801784
*/
1781-
if (GinIsNewKey(scan))
1782-
ginNewScanKey(scan);
1785+
ginFreeScanKeys(so); /* there should be no keys yet, but just to be sure */
1786+
ginNewScanKey(scan);
17831787

17841788
if (GinIsVoidRes(scan))
17851789
PG_RETURN_INT64(0);

src/backend/access/gin/ginscan.c

+12-4
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,10 @@ ginFillScanKey(GinScanOpaque so, OffsetNumber attnum,
163163
key->curItemMatches = false;
164164
key->recheckCurItem = false;
165165
key->isFinished = false;
166+
key->nrequired = 0;
167+
key->nadditional = 0;
168+
key->requiredEntries = NULL;
169+
key->additionalEntries = NULL;
166170

167171
ginInitConsistentFunction(ginstate, key);
168172

@@ -223,8 +227,8 @@ ginFillScanKey(GinScanOpaque so, OffsetNumber attnum,
223227
}
224228
}
225229

226-
static void
227-
freeScanKeys(GinScanOpaque so)
230+
void
231+
ginFreeScanKeys(GinScanOpaque so)
228232
{
229233
uint32 i;
230234

@@ -237,6 +241,10 @@ freeScanKeys(GinScanOpaque so)
237241

238242
pfree(key->scanEntry);
239243
pfree(key->entryRes);
244+
if (key->requiredEntries)
245+
pfree(key->requiredEntries);
246+
if (key->additionalEntries)
247+
pfree(key->additionalEntries);
240248
}
241249

242250
pfree(so->keys);
@@ -416,7 +424,7 @@ ginrescan(PG_FUNCTION_ARGS)
416424
/* remaining arguments are ignored */
417425
GinScanOpaque so = (GinScanOpaque) scan->opaque;
418426

419-
freeScanKeys(so);
427+
ginFreeScanKeys(so);
420428

421429
if (scankey && scan->numberOfKeys > 0)
422430
{
@@ -434,7 +442,7 @@ ginendscan(PG_FUNCTION_ARGS)
434442
IndexScanDesc scan = (IndexScanDesc) PG_GETARG_POINTER(0);
435443
GinScanOpaque so = (GinScanOpaque) scan->opaque;
436444

437-
freeScanKeys(so);
445+
ginFreeScanKeys(so);
438446

439447
MemoryContextDelete(so->tempCtx);
440448

src/include/access/gin_private.h

+1
Original file line numberDiff line numberDiff line change
@@ -890,6 +890,7 @@ extern Datum ginrescan(PG_FUNCTION_ARGS);
890890
extern Datum ginmarkpos(PG_FUNCTION_ARGS);
891891
extern Datum ginrestrpos(PG_FUNCTION_ARGS);
892892
extern void ginNewScanKey(IndexScanDesc scan);
893+
extern void ginFreeScanKeys(GinScanOpaque so);
893894

894895
/* ginget.c */
895896
extern Datum gingetbitmap(PG_FUNCTION_ARGS);

0 commit comments

Comments
 (0)