From 7cd23aad2a7aeeac276019e2f831313c75a8be2a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 30 Mar 2026 13:59:54 -0400 Subject: [PATCH] Be more careful to preserve consistency of a tuplestore. Several places in tuplestore.c would leave the tuplestore data structure effectively corrupt if some subroutine were to throw an error. Notably, if WRITETUP() failed after some number of successful calls within dumptuples(), the tuplestore would contain some memtuples pointers that were apparently live entries but in fact pointed to pfree'd chunks. In most cases this sort of thing is fine because transaction abort cleanup is not too picky about the contents of memory that it's going to throw away anyway. There's at least one exception though: if a Portal has a holdStore, we're going to call tuplestore_end() on that, even during transaction abort. So it's not cool if that tuplestore is corrupt, and that means tuplestore.c has to be more careful. This oversight demonstrably leads to crashes in v15 and before, if a holdable cursor fails to persist its data due to an undersized temp_file_limit setting. Very possibly the same thing can happen in v16 and v17 as well, though the specific test case submitted failed to fail there (cf. 095555daf). The failure is accidentally dodged as of v18 because 590b045c3 got rid of tuplestore_end's retail tuple deletion loop. Still, it seems unwise to permit tuplestores to become internally inconsistent in any branch, so I've applied the same fix across the board. Since the known test case for this is rather expensive and doesn't fail in recent branches, I've omitted it. Bug: #19438 Reported-by: Dmitriy Kuzmin Author: Tom Lane Reviewed-by: David Rowley Discussion: https://postgr.es/m/19438-9d37b179c56d43aa@postgresql.org Backpatch-through: 14 --- src/backend/utils/sort/tuplestore.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/backend/utils/sort/tuplestore.c b/src/backend/utils/sort/tuplestore.c index 509a91b503e..167a5d354c3 100644 --- a/src/backend/utils/sort/tuplestore.c +++ b/src/backend/utils/sort/tuplestore.c @@ -673,10 +673,10 @@ grow_memtuples(Tuplestorestate *state) /* OK, do it */ FREEMEM(state, GetMemoryChunkSpace(state->memtuples)); - state->memtupsize = newmemtupsize; state->memtuples = (void **) repalloc_huge(state->memtuples, - state->memtupsize * sizeof(void *)); + newmemtupsize * sizeof(void *)); + state->memtupsize = newmemtupsize; USEMEM(state, GetMemoryChunkSpace(state->memtuples)); if (LACKMEM(state)) elog(ERROR, "unexpected out-of-memory situation in tuplestore"); @@ -1221,7 +1221,19 @@ dumptuples(Tuplestorestate *state) if (i >= state->memtupcount) break; WRITETUP(state, state->memtuples[i]); + + /* + * Increase memtupdeleted to track the fact that we just deleted that + * tuple. Think not to remove this on the grounds that we'll reset + * memtupdeleted to zero below. We might not reach that if some later + * WRITETUP fails (e.g. due to overrunning temp_file_limit). If so, + * we'd error out leaving an effectively-corrupt tuplestore, which + * would be quite bad if it's a persistent data structure such as a + * Portal's holdStore. + */ + state->memtupdeleted++; } + /* Now we can reset memtupdeleted along with memtupcount */ state->memtupdeleted = 0; state->memtupcount = 0; } @@ -1408,8 +1420,10 @@ tuplestore_trim(Tuplestorestate *state) FREEMEM(state, GetMemoryChunkSpace(state->memtuples[i])); pfree(state->memtuples[i]); state->memtuples[i] = NULL; + /* As in dumptuples(), increment memtupdeleted synchronously */ + state->memtupdeleted++; } - state->memtupdeleted = nremove; + Assert(state->memtupdeleted == nremove); /* mark tuplestore as truncated (used for Assert crosschecks only) */ state->truncated = true;