From 6a12161c17052601ec7e7a398ec691adec7f48fb Mon Sep 17 00:00:00 2001 From: Pavel Borisov Date: Mon, 2 Nov 2020 20:07:06 +0400 Subject: [PATCH v2] Fix for GiST buffered build bug related to context resets inside a calls tree which can render itup pointer invalid --- src/backend/access/gist/gistbuild.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c index 9d3fa9c3b7..94d65b39d8 100644 --- a/src/backend/access/gist/gistbuild.c +++ b/src/backend/access/gist/gistbuild.c @@ -818,10 +818,24 @@ gistBuildCallback(Relation index, true); itup->t_tid = *tid; + /* Update tuple count and total size. */ + buildstate->indtuples += 1; + buildstate->indtuplesSize += IndexTupleSize(itup); + + /* + * XXX Generally, memory context better be 'owned' by only one function. + * If the context is reset somewhere inside its calls, pointers can + * become dangling just immediately. Though in the case of + * gistBufferingBuildInsert, tempCxt can be reset inside + * gistProcessEmptyingQueue because it is the last operation inside the + * call and no pointers are used after it, so we can allow this piece of + * magic. + */ if (buildstate->buildMode == GIST_BUFFERING_ACTIVE) { /* We have buffers, so use them. */ gistBufferingBuildInsert(buildstate, itup); + /* tempCxt is already reset in gistProcessEmptyingQueue */ } else { @@ -831,14 +845,9 @@ gistBuildCallback(Relation index, */ gistdoinsert(index, itup, buildstate->freespace, buildstate->giststate, buildstate->heaprel, true); + MemoryContextReset(buildstate->giststate->tempCxt); } - - /* Update tuple count and total size. */ - buildstate->indtuples += 1; - buildstate->indtuplesSize += IndexTupleSize(itup); - MemoryContextSwitchTo(oldCtx); - MemoryContextReset(buildstate->giststate->tempCxt); if (buildstate->buildMode == GIST_BUFFERING_ACTIVE && buildstate->indtuples % BUFFERING_MODE_TUPLE_SIZE_STATS_TARGET == 0) -- 2.28.0