aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2016-09-06 15:50:31 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2016-09-06 15:50:31 -0400
commitf032722f86a709277d44a317a3bc8acd74861a72 (patch)
tree81bafcfaa196e6e9ee3bf222da95e9e3dfb9984e
parentcdc70597c9ba62aad08a46e55c0c783bf4c21c9c (diff)
downloadpostgresql-f032722f86a709277d44a317a3bc8acd74861a72.tar.gz
postgresql-f032722f86a709277d44a317a3bc8acd74861a72.zip
Guard against possible memory allocation botch in batchmemtuples().
Negative availMemLessRefund would be problematic. It's not entirely clear whether the case can be hit in the code as it stands, but this seems like good future-proofing in any case. While we're at it, insist that the value be not merely positive but not tiny, so as to avoid doing a lot of repalloc work for little gain. Peter Geoghegan Discussion: <CAM3SWZRVkuUB68DbAkgw=532gW0f+fofKueAMsY7hVYi68MuYQ@mail.gmail.com>
-rw-r--r--src/backend/utils/sort/tuplesort.c23
1 files changed, 21 insertions, 2 deletions
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index c8fbcf8fcc9..aa8e0e42fc3 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -2866,6 +2866,9 @@ batchmemtuples(Tuplesortstate *state)
int64 availMemLessRefund;
int memtupsize = state->memtupsize;
+ /* Caller error if we have no tapes */
+ Assert(state->activeTapes > 0);
+
/* For simplicity, assume no memtuples are actually currently counted */
Assert(state->memtupcount == 0);
@@ -2880,6 +2883,20 @@ batchmemtuples(Tuplesortstate *state)
availMemLessRefund = state->availMem - refund;
/*
+ * We need to be sure that we do not cause LACKMEM to become true, else
+ * the batch allocation size could be calculated as negative, causing
+ * havoc. Hence, if availMemLessRefund is negative at this point, we must
+ * do nothing. Moreover, if it's positive but rather small, there's
+ * little point in proceeding because we could only increase memtuples by
+ * a small amount, not worth the cost of the repalloc's. We somewhat
+ * arbitrarily set the threshold at ALLOCSET_DEFAULT_INITSIZE per tape.
+ * (Note that this does not represent any assumption about tuple sizes.)
+ */
+ if (availMemLessRefund <=
+ (int64) state->activeTapes * ALLOCSET_DEFAULT_INITSIZE)
+ return;
+
+ /*
* To establish balanced memory use after refunding palloc overhead,
* temporarily have our accounting indicate that we've allocated all
* memory we're allowed to less that refund, and call grow_memtuples() to
@@ -2888,9 +2905,11 @@ batchmemtuples(Tuplesortstate *state)
state->growmemtuples = true;
USEMEM(state, availMemLessRefund);
(void) grow_memtuples(state);
- /* Should not matter, but be tidy */
- FREEMEM(state, availMemLessRefund);
state->growmemtuples = false;
+ /* availMem must stay accurate for spacePerTape calculation */
+ FREEMEM(state, availMemLessRefund);
+ if (LACKMEM(state))
+ elog(ERROR, "unexpected out-of-memory situation in tuplesort");
#ifdef TRACE_SORT
if (trace_sort)