Robert Haas <robertmhaas@gmail.com> writes:
> What I'm concerned about is that I think that (as I said on the other
> thread) is that ProcessGetMemoryContextInterrupt is not really at all
> safe to execute at an arbitrary CHECK_FOR_INTERRUPTS().
I agree.
> In my mind, the possible fixes here are (1) revert that patch, (2)
> redesign things so that the problematic code can only get called when
> we know that the backend state is sane, or (3) redesign the code so
> that it has fewer requirements for correct operation.
I want to argue for reverting, at least for v18. I do not think that
ProcessGetMemoryContextInterrupt is anywhere near release-quality.
I found out while poking into Valgrind leak reports that it leaks
memory --- and does so in TopMemoryContext. This is especially
unfortunate for something that's supposed to be used to investigate
memory consumption: a tool that affects the results under
consideration is not a great tool. The way I'd build it is to make
a special-purpose context that is a top-level context in its own
right (ie not a child of TopMemoryContext) and do all the work
therein, then reset or delete that context on the way out.
I also think it's remarkably poor design to have shoved this code
into mcxt.c, which is a very low-level module that has no business
having the #include dependencies that it acquired in 042a66291.
We've put some other stuff in mcxt.c that probably shouldn't be
there either, but bloating the file by 40% is a bit much.
regards, tom lane
