Re: BUG #15703: Segfault in cancelled CALL-Statements

I wrote:
>> I find HoldPinnedPortals suspicious in another way: is it really
>> OK for it to mark *all* pinned portals as auto-held? What if we're
>> in a nest of procedures run by different PLs (of which only the
>> innermost will have any idea we're committing)?

> After taking another look at this, I'm convinced that it's flat out
> broken. It cannot be up to individual PLs to decide whether or not
> to call HoldPinnedPortals before issuing a commit. If a PL that needs
> that calls (a function belonging to) some other PL that thinks it
> doesn't need that, and the second PL issues a commit or rollback,
> we'll have a failure for whatever portal(s) the first PL left pinned.

After further study, it seems like the most practical thing to do here
is to assign the responsibility for calling HoldPinnedPortals to spi.c.

If there are any PLs out there that are trying to implement commit/
rollback in procedures without using SPI, it's going to be on their
heads to include this step --- but there's a lot of other stuff they
are going to need to know if they don't go through SPI, so that doesn't
seem too awful, and in any case this change doesn't make it any more
complicated for them.

Conversely, if there are any PLs out there that are using spi.c and
following the existing rule that they should call HoldPinnedPortals for
themselves, this won't break them, because running HoldPinnedPortals
twice in a row is harmless.

Accordingly I propose the attached patch. Aside from moving those
calls, adjusting the comments, and fixing the original bug, this
inserts a test on portal->status, which I think is necessary by
analogy to the existing tests in AtCommit_Portals.

regards, tom lane