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

Commit 6a17826

Browse files
committed
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.
1 parent b2facfd commit 6a17826

File tree

1 file changed

+20
-3
lines changed

1 file changed

+20
-3
lines changed

src/backend/access/hash/hashscan.c

+20-3
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,28 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/hash/hashscan.c,v 1.43 2008/01/01 19:45:46 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/hash/hashscan.c,v 1.44 2008/03/07 15:59:03 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
1515

1616
#include "postgres.h"
1717

1818
#include "access/hash.h"
19+
#include "utils/memutils.h"
1920
#include "utils/resowner.h"
2021

2122

23+
/*
24+
* We track all of a backend's active scans on hash indexes using a list
25+
* of HashScanListData structs, which are allocated in TopMemoryContext.
26+
* It's okay to use a long-lived context because we rely on the ResourceOwner
27+
* mechanism to clean up unused entries after transaction or subtransaction
28+
* abort. We can't safely keep the entries in the executor's per-query
29+
* context, because that might be already freed before we get a chance to
30+
* clean up the list. (XXX seems like there should be a better way to
31+
* manage this...)
32+
*/
2233
typedef struct HashScanListData
2334
{
2435
IndexScanDesc hashsl_scan;
@@ -44,6 +55,11 @@ ReleaseResources_hash(void)
4455
HashScanList next;
4556

4657
/*
58+
* Release all HashScanList items belonging to the current ResourceOwner.
59+
* Note that we do not release the underlying IndexScanDesc; that's in
60+
* executor memory and will go away on its own (in fact quite possibly
61+
* has gone away already, so we mustn't try to touch it here).
62+
*
4763
* Note: this should be a no-op during normal query shutdown. However, in
4864
* an abort situation ExecutorEnd is not called and so there may be open
4965
* index scans to clean up.
@@ -69,14 +85,15 @@ ReleaseResources_hash(void)
6985
}
7086

7187
/*
72-
* _Hash_regscan() -- register a new scan.
88+
* _hash_regscan() -- register a new scan.
7389
*/
7490
void
7591
_hash_regscan(IndexScanDesc scan)
7692
{
7793
HashScanList new_el;
7894

79-
new_el = (HashScanList) palloc(sizeof(HashScanListData));
95+
new_el = (HashScanList) MemoryContextAlloc(TopMemoryContext,
96+
sizeof(HashScanListData));
8097
new_el->hashsl_scan = scan;
8198
new_el->hashsl_owner = CurrentResourceOwner;
8299
new_el->hashsl_next = HashScans;

0 commit comments

Comments
 (0)