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