From 13e4d0b8d381c1d0e507a09e2350150d4f1bc671 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Wed, 6 Sep 2023 17:53:16 +0900 Subject: [PATCH v47 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. Reviewed-by: Robert Haas --- src/backend/executor/nodeBitmapHeapscan.c | 3 ++- src/backend/executor/nodeForeignscan.c | 13 +++++++----- src/backend/executor/nodeIncrementalSort.c | 6 ++++-- src/backend/executor/nodeMemoize.c | 1 + src/backend/executor/nodeRecursiveunion.c | 6 ++++-- src/backend/executor/nodeWindowAgg.c | 24 ++++++++++++++-------- 6 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 2db0acfc76..c8c1c9d88e 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 != NULL) + table_endscan(scanDesc); } /* ---------------------------------------------------------------- diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c index 73913ebb18..298ea59a1e 100644 --- a/src/backend/executor/nodeForeignscan.c +++ b/src/backend/executor/nodeForeignscan.c @@ -301,13 +301,16 @@ ExecEndForeignScan(ForeignScanState *node) EState *estate = node->ss.ps.state; /* Let the FDW shut down */ - if (plan->operation != CMD_SELECT) + if (node->fdwroutine != NULL) { - 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)) diff --git a/src/backend/executor/nodeIncrementalSort.c b/src/backend/executor/nodeIncrementalSort.c index cd094a190c..544e64dfab 100644 --- a/src/backend/executor/nodeIncrementalSort.c +++ b/src/backend/executor/nodeIncrementalSort.c @@ -1079,8 +1079,10 @@ ExecEndIncrementalSort(IncrementalSortState *node) { SO_printf("ExecEndIncrementalSort: shutting down sort node\n"); - ExecDropSingleTupleTableSlot(node->group_pivot); - ExecDropSingleTupleTableSlot(node->transfer_tuple); + if (node->group_pivot != NULL) + ExecDropSingleTupleTableSlot(node->group_pivot); + if (node->transfer_tuple != NULL) + ExecDropSingleTupleTableSlot(node->transfer_tuple); /* * Release tuplesort resources. diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.c index 94bf479287..81f2acde5e 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 != NULL) { int count; uint64 mem = 0; diff --git a/src/backend/executor/nodeRecursiveunion.c b/src/backend/executor/nodeRecursiveunion.c index e781003934..54cd6f2347 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 != NULL) + tuplestore_end(node->working_table); + if (node->intermediate_table != NULL) + 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..f5170799e4 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 != NULL) + MemoryContextResetAndDeleteChildren(winstate->partcontext); + if (winstate->aggcontext != NULL) + MemoryContextResetAndDeleteChildren(winstate->aggcontext); for (i = 0; i < winstate->numaggs; i++) { - if (winstate->peragg[i].aggcontext != winstate->aggcontext) + if (winstate->peragg[i].aggcontext != NULL && + 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 != NULL && + node->peragg[i].aggcontext != node->aggcontext) MemoryContextDelete(node->peragg[i].aggcontext); } - MemoryContextDelete(node->partcontext); - MemoryContextDelete(node->aggcontext); + if (node->partcontext != NULL) + MemoryContextDelete(node->partcontext); + if (node->aggcontext != NULL) + MemoryContextDelete(node->aggcontext); - pfree(node->perfunc); - pfree(node->peragg); + if (node->perfunc != NULL) + pfree(node->perfunc); + if (node->peragg != NULL) + pfree(node->peragg); outerPlan = outerPlanState(node); ExecEndNode(outerPlan); -- 2.35.3