Fix hash_create calls without HASH_CONTEXT flag

This patch fixes callsites that set an explicit memory context
in the control structure but do not specify the HASH_CONTEXT flag
leading to the hash table being created in TopMemoryContext.
This patch also changes call sites that want to create the hash
table in TopMemoryContext to be explicit about this.
Additionally this patch adds a coccinelle script to detect these
errors and prevent adding similar code in the future.
This commit is contained in:
Sven Klemm 2022-07-31 17:43:04 +02:00 committed by Sven Klemm
parent 28440b7900
commit 336f4b513f
4 changed files with 51 additions and 6 deletions

View File

@ -0,0 +1,38 @@
// find hash_create calls without HASH_CONTEXT flag
//
// hash_create calls without HASH_CONTEXT flag will create the hash table in
// TopMemoryContext which can introduce memory leaks if not intended. We want
// to be explicit about the memory context our hash tables live in so we enforce
// usage of the flag.
@ hash_create @
expression res;
position p;
@@
res@p = hash_create(...);
@safelist@
expression res;
expression arg1, arg2, arg3;
expression w1, w2;
expression flags;
position hash_create.p;
@@
(
res@p = hash_create(arg1,arg2,arg3, w1 | HASH_CONTEXT | w2);
|
res@p = hash_create(arg1,arg2,arg3, w1 | HASH_CONTEXT);
|
res@p = hash_create(arg1,arg2,arg3, HASH_CONTEXT | w2 );
|
Assert(flags & HASH_CONTEXT);
res@p = hash_create(arg1,arg2,arg3, flags);
)
@ depends on !safelist @
expression res;
position hash_create.p;
@@
+ /* hash_create without HASH_CONTEXT flag */
res@p = hash_create(...);

View File

@ -48,6 +48,11 @@ ts_cache_init(Cache *cache)
*/
Assert(MemoryContextContains(ts_cache_memory_ctx(cache), cache));
/*
* We always want to be explicit about the memory context our hash table
* ends up in to ensure it's not accidently put in TopMemoryContext.
*/
Assert(cache->flags & HASH_CONTEXT);
cache->htab = hash_create(cache->name, cache->numelements, &cache->hctl, cache->flags);
cache->refcount = 1;
cache->handle_txn_callbacks = true;

View File

@ -241,7 +241,7 @@ data_node_chunk_assignments_are_overlapping(DataNodeChunkAssignments *scas,
all_data_node_slice_htab = hash_create("all_data_node_slices",
scas->total_num_chunks,
&hashctl,
HASH_ELEM | HASH_BLOBS);
HASH_BLOBS | HASH_CONTEXT | HASH_ELEM);
hash_seq_init(&status, scas->assignments);

View File

@ -102,13 +102,15 @@ InvalidateShippableCacheCallback(Datum arg, int cacheid, uint32 hashvalue)
static void
InitializeShippableCache(void)
{
HASHCTL ctl;
HASHCTL ctl = {
.keysize = sizeof(ShippableCacheKey),
.entrysize = sizeof(ShippableCacheEntry),
.hcxt = TopMemoryContext,
};
/* Create the hash table. */
MemSet(&ctl, 0, sizeof(ctl));
ctl.keysize = sizeof(ShippableCacheKey);
ctl.entrysize = sizeof(ShippableCacheEntry);
ShippableCacheHash = hash_create("Shippability cache", 256, &ctl, HASH_ELEM | HASH_BLOBS);
ShippableCacheHash =
hash_create("Shippability cache", 256, &ctl, HASH_BLOBS | HASH_CONTEXT | HASH_ELEM);
/* Set up invalidation callback on pg_foreign_server. */
CacheRegisterSyscacheCallback(FOREIGNSERVEROID, InvalidateShippableCacheCallback, (Datum) 0);