Julien Rouhaud <rjuju...@gmail.com> writes:
> On Wed, Oct 12, 2022 at 07:24:53PM -0400, Tom Lane wrote:
>> There are hundreds, if not thousands, of "shouldn't ever happen" elogs
>> in Postgres. We don't make any attempt to trap any of them. Why do
>> you think this one should be different?
> Because session variables are allocated in a persistent memory context, so
> there's a code doing something like this to implement LET variable:
> [...]
> oldctxt = MemoryContextSwitchTo(SomePersistentContext);
> newval = palloc(...);
> MemoryContextSwitchTo(oldctxt);
> /* No error should happen after that point or we leak memory */
> pfree(var->val);
> var->val = newval;
> return;
> Any error thrown in pfree would mean leaking memory forever in that backend.
I've got little sympathy for that complaint, because an error
in pfree likely means you have worse problems than a leak.
Moreover, what it most likely means is that you messed up your
memory allocations sometime earlier; making your code more
complicated makes that risk higher not lower.
Having said that, the above is really not very good code.
Better would look like
newval = MemoryContextAlloc(SomePersistentContext, ...);
... fill newval ...
oldval = var->val;
var->val = newval;
pfree(oldval);
which leaves your data structure in a consistent state whether
pfree fails or not (and regardless of whether it fails before or
after recycling your chunk). That property is way more important
than the possibility of leaking some memory.
regards, tom lane
