From 3dfe81f48a58a92e8c81469600d3502f18a8b137 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Fri, 1 Sep 2023 22:05:35 +0900 Subject: [PATCH v46 2/8] Check pointer NULLness before cleanup in ExecEnd* routines Many routines already perform this check, but a few instances remain. Currently, these NULLness checks might seem redundant since ExecEnd* routines operate under the assumption that their matching ExecInit* routine would have fully executed, ensuring pointers are set. However, a forthcoming patch will modify ExecInit* routines to sometimes exit early, potentially leaving some pointers in an undetermined state. --- src/backend/executor/nodeAgg.c | 3 ++- src/backend/executor/nodeBitmapHeapscan.c | 3 ++- src/backend/executor/nodeForeignscan.c | 21 ++++++++------------ src/backend/executor/nodeMemoize.c | 1 + src/backend/executor/nodeRecursiveunion.c | 6 ++++-- src/backend/executor/nodeWindowAgg.c | 24 +++++++++++++++-------- 6 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index f154f28902..aac9e9fc80 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -3150,7 +3150,8 @@ hashagg_reset_spill_state(AggState *aggstate) } /* free batches */ - list_free_deep(aggstate->hash_batches); + if (aggstate->hash_batches) + list_free_deep(aggstate->hash_batches); aggstate->hash_batches = NIL; /* close tape set */ diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 2db0acfc76..ffa51c06b4 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -681,7 +681,8 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node) /* * close heap scan */ - table_endscan(scanDesc); + if (scanDesc) + table_endscan(scanDesc); } /* ---------------------------------------------------------------- diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c index c2139acca0..d5aaa983f7 100644 --- a/src/backend/executor/nodeForeignscan.c +++ b/src/backend/executor/nodeForeignscan.c @@ -301,25 +301,20 @@ ExecEndForeignScan(ForeignScanState *node) EState *estate = node->ss.ps.state; /* Let the FDW shut down */ - if (plan->operation != CMD_SELECT) + if (node->fdwroutine) { - if (estate->es_epq_active == NULL) - node->fdwroutine->EndDirectModify(node); + if (plan->operation != CMD_SELECT) + { + if (estate->es_epq_active == NULL) + node->fdwroutine->EndDirectModify(node); + } + else + node->fdwroutine->EndForeignScan(node); } - else - node->fdwroutine->EndForeignScan(node); /* Shut down any outer plan. */ if (outerPlanState(node)) ExecEndNode(outerPlanState(node)); - - /* Free the exprcontext */ - ExecFreeExprContext(&node->ss.ps); - - /* clean out the tuple table */ - if (node->ss.ps.ps_ResultTupleSlot) - ExecClearTuple(node->ss.ps.ps_ResultTupleSlot); - ExecClearTuple(node->ss.ss_ScanTupleSlot); } /* ---------------------------------------------------------------- diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.c index 94bf479287..5352ca10c8 100644 --- a/src/backend/executor/nodeMemoize.c +++ b/src/backend/executor/nodeMemoize.c @@ -1043,6 +1043,7 @@ ExecEndMemoize(MemoizeState *node) { #ifdef USE_ASSERT_CHECKING /* Validate the memory accounting code is correct in assert builds. */ + if (node->hashtable) { int count; uint64 mem = 0; diff --git a/src/backend/executor/nodeRecursiveunion.c b/src/backend/executor/nodeRecursiveunion.c index e781003934..3dfcb4cafb 100644 --- a/src/backend/executor/nodeRecursiveunion.c +++ b/src/backend/executor/nodeRecursiveunion.c @@ -272,8 +272,10 @@ void ExecEndRecursiveUnion(RecursiveUnionState *node) { /* Release tuplestores */ - tuplestore_end(node->working_table); - tuplestore_end(node->intermediate_table); + if (node->working_table) + tuplestore_end(node->working_table); + if (node->intermediate_table) + tuplestore_end(node->intermediate_table); /* free subsidiary stuff including hashtable */ if (node->tempContext) diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 77724a6daa..3849d2f847 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -1351,11 +1351,14 @@ release_partition(WindowAggState *winstate) * any aggregate temp data). We don't rely on retail pfree because some * aggregates might have allocated data we don't have direct pointers to. */ - MemoryContextResetAndDeleteChildren(winstate->partcontext); - MemoryContextResetAndDeleteChildren(winstate->aggcontext); + if (winstate->partcontext) + MemoryContextResetAndDeleteChildren(winstate->partcontext); + if (winstate->aggcontext) + MemoryContextResetAndDeleteChildren(winstate->aggcontext); for (i = 0; i < winstate->numaggs; i++) { - if (winstate->peragg[i].aggcontext != winstate->aggcontext) + if (winstate->peragg[i].aggcontext && + winstate->peragg[i].aggcontext != winstate->aggcontext) MemoryContextResetAndDeleteChildren(winstate->peragg[i].aggcontext); } @@ -2688,14 +2691,19 @@ ExecEndWindowAgg(WindowAggState *node) for (i = 0; i < node->numaggs; i++) { - if (node->peragg[i].aggcontext != node->aggcontext) + if (node->peragg[i].aggcontext && + node->peragg[i].aggcontext != node->aggcontext) MemoryContextDelete(node->peragg[i].aggcontext); } - MemoryContextDelete(node->partcontext); - MemoryContextDelete(node->aggcontext); + if (node->partcontext) + MemoryContextDelete(node->partcontext); + if (node->aggcontext) + MemoryContextDelete(node->aggcontext); - pfree(node->perfunc); - pfree(node->peragg); + if (node->perfunc) + pfree(node->perfunc); + if (node->peragg) + pfree(node->peragg); outerPlan = outerPlanState(node); ExecEndNode(outerPlan); -- 2.35.3