Re: Renaming MemoryContextResetAndDeleteChildren to MemoryContextReset

Andres Freund <andres(at)2ndquadrant(dot)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(dot)16321(dot)37(dot)camel(at)dell(dot)linuxdev(dot)us(dot)dell(dot)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