Rahila Syed <rahilasye...@gmail.com> writes:
> +1 for adding the assertion to increase the chances of this bug being
> caught by memory context infrastructure.
I verified that an assertion inside MemoryContextCreate would catch
this bug, so I added one (in 5ec8b01c3), and then pushed the main
fix at 80b727eb9.
> I had the following comment.
> Why do we do this:
> - MemoryContext old_context;
> + MemoryContext old_context = CurrentMemoryContext;
> Instead of implementing it as done in the previous version of this code,
> i.e.
> old_context = MemoryContextSwitchTo(cmd_context);
Because capturing old_context at that point is too late. The crux
of the bug is exactly that SnapBuildClearExportedSnapshot may have
changed CurrentMemoryContext. We could have avoided that by
reordering the code, or done something like what Anthonin did in v02.
But this way is perfectly idiomatic IMO. There are some dozens of
similar usages elsewhere in our code, and it makes very sure that
old_context captures the caller's context even if we rearrange things
some more inside exec_replication_command.
regards, tom lane
