pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: julian.schauder@gmx.de
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15703: Segfault in cancelled CALL-Statements
Date: Mon, 25 Mar 2019 13:36:14 -0400
Message-ID: <20459.1553535374@sss.pgh.pa.us> (raw)
In-Reply-To: <8678.1553097373@sss.pgh.pa.us>
References: <15703-c12c5bc0ea34ba26@postgresql.org>
	<8678.1553097373@sss.pgh.pa.us>

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.

I think, therefore, that we should get rid of HoldPinnedPortals and
just assign the responsibility for holding pinned portals to the
normal code paths, probably PreCommit_Portals and AtAbort_Portals.

Another issue in this area is whether it's really safe to hold a portal
during transaction abort at all.  I'm inclined to think that it is not.
HoldPortal can call arbitrary user code, and the idea that we'll let that
happen during an already-failed transaction seems borderline insane from
here.  However, I'm not very sure what we can do instead.

As things stand right now, the calls of HoldPinnedPortals from places
like exec_stmt_rollback are safe in this sense, because we haven't yet
started to abort the current transaction.  However, that just raises
the question of why we need those calls at all.  If a transaction
abort happens due to some kind of error, we aren't going to do any
pinned-portal holding, so why should we need it in exec_stmt_rollback?

Maybe the rule needs to be "we'll do HoldPinnedPortals in SPI_rollback,
but not if you reach transaction abort via an actual error".  Seems
a bit weird though.

Thoughts?

			regards, tom lane




view thread (7+ messages)  latest in thread

Message-ID: <20459.1553535374@sss.pgh.pa.us>
Permalink:  ../20459.1553535374@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/20459.1553535374@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: tgl@sss.pgh.pa.us, peter.eisentraut@2ndquadrant.com, julian.schauder@gmx.de, pgsql-bugs@lists.postgresql.org
  Subject: Re: BUG #15703: Segfault in cancelled CALL-Statements
  In-Reply-To: <20459.1553535374@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox