From 6a178266210376835a7fb0de71bc150c8bb852e7 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 7 Mar 2008 15:59:03 +0000
Subject: [PATCH] Change hashscan.c to keep its list of active hash index scans
in TopMemoryContext, rather than scattered through executor per-query
contexts. This poses no danger of memory leak since the ResourceOwner
mechanism guarantees release of no-longer-needed items. It is needed because
the per-query context might already be released by the time we try to clean
up the hash scan list. Report by ykhuang, diagnosis by Heikki.
Back-patch to 8.0, where the ResourceOwner-based cleanup was introduced.
The given test case does not fail before 8.2, probably because we rearranged
transaction abort processing somehow; but this coding is undoubtedly risky
so I'll patch 8.0 and 8.1 anyway.
---
src/backend/access/hash/hashscan.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/hash/hashscan.c b/src/backend/access/hash/hashscan.c
index 2598c5fb0b1..1272ec95acb 100644
--- a/src/backend/access/hash/hashscan.c
+++ b/src/backend/access/hash/hashscan.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/access/hash/hashscan.c,v 1.43 2008/01/01 19:45:46 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/access/hash/hashscan.c,v 1.44 2008/03/07 15:59:03 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -16,9 +16,20 @@
#include "postgres.h"
#include "access/hash.h"
+#include "utils/memutils.h"
#include "utils/resowner.h"
+/*
+ * We track all of a backend's active scans on hash indexes using a list
+ * of HashScanListData structs, which are allocated in TopMemoryContext.
+ * It's okay to use a long-lived context because we rely on the ResourceOwner
+ * mechanism to clean up unused entries after transaction or subtransaction
+ * abort. We can't safely keep the entries in the executor's per-query
+ * context, because that might be already freed before we get a chance to
+ * clean up the list. (XXX seems like there should be a better way to
+ * manage this...)
+ */
typedef struct HashScanListData
{
IndexScanDesc hashsl_scan;
@@ -44,6 +55,11 @@ ReleaseResources_hash(void)
HashScanList next;
/*
+ * Release all HashScanList items belonging to the current ResourceOwner.
+ * Note that we do not release the underlying IndexScanDesc; that's in
+ * executor memory and will go away on its own (in fact quite possibly
+ * has gone away already, so we mustn't try to touch it here).
+ *
* Note: this should be a no-op during normal query shutdown. However, in
* an abort situation ExecutorEnd is not called and so there may be open
* index scans to clean up.
@@ -69,14 +85,15 @@ ReleaseResources_hash(void)
}
/*
- * _Hash_regscan() -- register a new scan.
+ * _hash_regscan() -- register a new scan.
*/
void
_hash_regscan(IndexScanDesc scan)
{
HashScanList new_el;
- new_el = (HashScanList) palloc(sizeof(HashScanListData));
+ new_el = (HashScanList) MemoryContextAlloc(TopMemoryContext,
+ sizeof(HashScanListData));
new_el->hashsl_scan = scan;
new_el->hashsl_owner = CurrentResourceOwner;
new_el->hashsl_next = HashScans;
--
GitLab