Re: AtAbort_Portsl problem

Tatsuo Ishii <ishii(at)postgresql(dot)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
count go 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