mirror of
https://github.com/postgres/postgres.git
synced 2026-04-07 10:17:24 -04:00
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 because590b045c3got 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 <kuzmin.db4@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/19438-9d37b179c56d43aa@postgresql.org Backpatch-through: 14
This commit is contained in:
parent
6ce5c310ba
commit
7cd23aad2a
1 changed files with 17 additions and 3 deletions
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Reference in a new issue