I believe this patch (which also fixes a comment I neglected to fix in
the previous one) should satisfy your concerns.
It's still running a few relevant tests (create_function_1 create_type
point polygon circle create_table copy create_misc create_index
alter_table partition_join partition_prune hash_part) under
CLOBBER_FREED_MEMORY + CLOBBER_CACHE_ALWAYS, but so far it's looking
good.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 8d72b371c9d46bf509e75ed8e33ac7d6bb4f1840 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Fri, 22 Dec 2017 13:19:32 -0300
Subject: [PATCH] Protect against hypothetical memory leaks in
RelationGetPartitionKey
---
src/backend/utils/cache/relcache.c | 40 ++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 17 deletions(-)
diff --git a/src/backend/utils/cache/relcache.c
b/src/backend/utils/cache/relcache.c
index e2760daac4..f25e9ba536 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -807,17 +807,16 @@ RelationBuildRuleLock(Relation relation)
* RelationBuildPartitionKey
* Build and attach to relcache partition key data of relation
*
- * Partitioning key data is stored in CacheMemoryContext to ensure it survives
- * as long as the relcache. To avoid leaking memory in that context in case
- * of an error partway through this function, we build the structure in the
- * working context (which must be short-lived) and copy the completed
- * structure into the cache memory.
- *
- * Also, since the structure being created here is sufficiently complex, we
- * make a private child context of CacheMemoryContext for each relation that
- * has associated partition key information. That means no complicated logic
- * to free individual elements whenever the relcache entry is flushed - just
- * delete the context.
+ * Partitioning key data is a complex structure; to avoid complicated logic to
+ * free individual elements whenever the relcache entry is flushed, we give it
+ * its own memory context, child of CacheMemoryContext, which can easily be
+ * deleted on its own. To avoid leaking memory in that context in case of an
+ * error partway through this function, the context is initially created as a
+ * child of CurTransactionContext and only re-parented to CacheMemoryContext
+ * at the end, when no further errors are possible. Also, we don't make this
+ * context the current context except in very brief code sections, out of fear
+ * that some of our callees allocate memory on their own which would be leaked
+ * permanently.
*/
static void
RelationBuildPartitionKey(Relation relation)
@@ -850,9 +849,9 @@ RelationBuildPartitionKey(Relation relation)
RelationGetRelationName(relation),
MEMCONTEXT_COPY_NAME,
ALLOCSET_SMALL_SIZES);
- oldcxt = MemoryContextSwitchTo(partkeycxt);
- key = (PartitionKey) palloc0(sizeof(PartitionKeyData));
+ key = (PartitionKey) MemoryContextAllocZero(partkeycxt,
+
sizeof(PartitionKeyData));
/* Fixed-length attributes */
form = (Form_pg_partitioned_table) GETSTRUCT(tuple);
@@ -900,11 +899,15 @@ RelationBuildPartitionKey(Relation relation)
*/
expr = eval_const_expressions(NULL, (Node *) expr);
+ oldcxt = MemoryContextSwitchTo(partkeycxt);
+ key->partexprs = (List *) copyObject(expr);
+ MemoryContextSwitchTo(oldcxt);
+
/* May as well fix opfuncids too */
- fix_opfuncids((Node *) expr);
- key->partexprs = (List *) expr;
+ fix_opfuncids((Node *) key->partexprs);
}
+ oldcxt = MemoryContextSwitchTo(partkeycxt);
key->partattrs = (AttrNumber *) palloc0(key->partnatts *
sizeof(AttrNumber));
key->partopfamily = (Oid *) palloc0(key->partnatts * sizeof(Oid));
key->partopcintype = (Oid *) palloc0(key->partnatts * sizeof(Oid));
@@ -919,6 +922,7 @@ RelationBuildPartitionKey(Relation relation)
key->parttypbyval = (bool *) palloc0(key->partnatts * sizeof(bool));
key->parttypalign = (char *) palloc0(key->partnatts * sizeof(char));
key->parttypcoll = (Oid *) palloc0(key->partnatts * sizeof(Oid));
+ MemoryContextSwitchTo(oldcxt);
/* For the hash partitioning, an extended hash function will be used. */
procnum = (key->strategy == PARTITION_STRATEGY_HASH) ?
@@ -989,11 +993,13 @@ RelationBuildPartitionKey(Relation relation)
ReleaseSysCache(tuple);
- /* Success --- make the relcache point to the newly constructed key */
+ /*
+ * Success --- reparent our context and make the relcache point to the
+ * newly constructed key
+ */
MemoryContextSetParent(partkeycxt, CacheMemoryContext);
relation->rd_partkeycxt = partkeycxt;
relation->rd_partkey = key;
- MemoryContextSwitchTo(oldcxt);
}
/*
--
2.11.0
