Re: Renaming MemoryContextResetAndDeleteChildren to MemoryContextReset - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Renaming MemoryContextResetAndDeleteChildren to MemoryContextReset
Date
Msg-id 30502.1424990726@sss.pgh.pa.us
Whole thread Raw
In response to Re: Renaming MemoryContextResetAndDeleteChildren to MemoryContextReset  (Andres Freund <andres@2ndquadrant.com>)
Responses Re: Renaming MemoryContextResetAndDeleteChildren to MemoryContextReset
List pgsql-hackers
Andres Freund <andres@2ndquadrant.com> writes:
> On 2015-02-26 17:01:53 -0500, Tom Lane wrote:
>> We've discussed doing $SUBJECT off and on for nearly ten years,
>> the oldest thread I could find about it being here:
>> http://www.postgresql.org/message-id/flat/1186435268.16321.37.camel@dell.linuxdev.us.dell.com
>> It's come up again every time we found another leak of dead child
>> contexts, which happened twice last year for example.  And that patch
>> I'm pushing for "expanded" out-of-line objects really needs this to
>> become the default behavior anywhere that we can detoast objects.

> I don't object to the behavioural change per se, rather the contrary,
> but I find it a pretty bad idea to change the meaning of an existing
> functioname this way.

That's pretty much the whole point I think.

Or are you arguing for an alternative proposal in which we remove
MemoryContextReset (or at least rename it to something new) and thereby
intentionally break all code that uses MemoryContextReset?  I can't say
that I find that an improvement.

> ... Without a compiler erroring out people won't
> notice that suddenly MemoryContextReset deletes much more; leading to
> possibly hard to find errors. Context resets frequently are in error
> paths, and those won't necessarily be hit when running with assertions
> enabled.

With all due respect, that's utterly wrong.  I have looked at every single
MemoryContextReset call in the codebase, and as far as I can see the
*only* one that is in an error path is elog.c:336, which I'm quite certain
is wrong anyway (the other reset of ErrorContext in that file uses
MemoryContextResetAndDeleteChildren, this one should too).

I see no reason whatever to believe that this change is likely to do
anything except fix heretofore-unnoticed memory leaks.
        regards, tom lane



pgsql-hackers by date:

Previous
From: Jim Nasby
Date:
Subject: Re: Precedence of standard comparison operators
Next
From: Andres Freund
Date:
Subject: Re: Renaming MemoryContextResetAndDeleteChildren to MemoryContextReset