On Thu, Apr 6, 2017 at 2:05 PM, Andres Freund <and...@anarazel.de> wrote: > I'm not sure you entirely got my point here. If tuplesort_gettupleslot > is called with copy = true, it'll store that tuple w/ > ExecStoreMinimalTuple passing shouldFree = copy = true. If the caller > is in a short-lived context, e.g. some expression context, and resets > that context before the slot is cleared (either by storing another tuple > or ExecClearTuple()) you'll get a double free, because the context has > freed the tuple in bulk, and then > if (slot->tts_shouldFree) > heap_freetuple(slot->tts_tuple); > will do its work.
Calling ExecClearTuple() will mark the slot "tts_shouldFree = false" -- you only have a problem when a memory context is reset, which obviously cannot be accounted for by ExecClearTuple(). ISTM that ResetExprContext() is kind of called to "make sure" that memory is freed in a short-lived expression context, at a level up from any ExecStoreMinimalTuple()/ExecClearTuple() style memory management. The conventions are not centrally documented, AFAIK, but this still seems natural enough to me. Per-tuple contexts tend to be associated with expression contexts. In any case, I'm not sure where you'd centrally document the conventions. Although, it seems clear that it wouldn't be anywhere this patch currently touches. The executor README, perhaps? -- Peter Geoghegan VMware vCenter Server https://www.vmware.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
