Re: AtAbort_Portsl problem

Tatsuo Ishii <ishii@postgresql.org> writes:
> The patch addresses the original issue.  The reason why you didn't see
> crash was just you were lucky, I believe. I'm sure that your
> exec_execute_message looks into already-freed-memory.

[ shrug... ]  If it did, it would have crashed, because I invariably
build with --cassert-enabled -> CLOBBER_FREED_MEMORY.

I do now see the risk path you are talking about, I think:

1. bind to some fully_planned prepared statement, causing the Portal  to link directly to a CachedPlan's statement
list;
2. invalidate the prepared statement, so that the CachedPlanSource  drops its reference to the CachedPlan;
3. AbortTransaction, so that AtAbort_Portals runs  PortalReleaseCachedPlan.  This makes the CachedPlan's reference
countgo to zero, so it drops its memory.  Now the Portal's  stmts pointer is dangling;
 
4. try to execute the Portal.

I do not believe it's possible to make this happen through libpq
alone, at least not without a great deal more hacking than you
did on it.  libpq doesn't ever put very much in between binding
a portal and executing it.  But a non-libpq-based client could
easily do it if it intermixed binding and executing a few different
portals.  Also step 2 might happen unluckily through a SI reset
triggered by some concurrent session.

I don't like the proposed patch though since it closes only one of
the paths by which a Portal might drop its reference to a CachedPlan.
I think the right place to clear portal->stmts is in
PortalReleaseCachedPlan.
        regards, tom lane