From 1c3a877469a93ae2e753da026bd1b939d8cf7f11 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 3 Feb 2021 19:38:29 -0500 Subject: [PATCH] Avoid crash when rolling back within a prepared statement. If a portal is used to run a prepared CALL or DO statement that contains a ROLLBACK, PortalRunMulti fails because the portal's statement list gets cleared by the rollback. (Since the grammar doesn't allow CALL/DO in PREPARE, the only easy way to get to this is via extended query protocol, which treats all inputs as prepared statements.) It's difficult to avoid resetting the portal early because of resource-management issues, so work around this by teaching PortalRunMulti to be wary of portal->stmts having suddenly become NIL. The crash has only been seen to occur in v13 and HEAD (as a consequence of commit 1cff1b95a having added an extra touch of portal->stmts). But even before that, the code involved touching a List that the portal no longer has any claim on. In the test case at hand, the List will still exist because of another refcount on the cached plan; but I'm far from convinced that it's impossible for the cached plan to have been dropped by the time control gets back to PortalRunMulti. Hence, backpatch to v11 where nested transactions were added. Thomas Munro and Tom Lane, per bug #16811 from James Inform Discussion: https://postgr.es/m/16811-c1b599b2c6c2d622@postgresql.org --- src/backend/tcop/pquery.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index 66cc5c35c6..78381a4472 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -1333,19 +1333,30 @@ PortalRunMulti(Portal portal, } } - /* - * Increment command counter between queries, but not after the last - * one. - */ - if (lnext(stmtlist_item) != NULL) - CommandCounterIncrement(); - /* * Clear subsidiary contexts to recover temporary memory. */ Assert(portal->portalContext == CurrentMemoryContext); MemoryContextDeleteChildren(portal->portalContext); + + /* + * Avoid crashing if portal->stmts has been reset. This can only + * occur if a CALL or DO utility statement executed an internal + * COMMIT/ROLLBACK (cf PortalReleaseCachedPlan). The CALL or DO must + * have been the only statement in the portal, so there's nothing left + * for us to do; but we don't want to dereference a now-dangling list + * pointer. + */ + if (portal->stmts == NIL) + break; + + /* + * Increment command counter between queries, but not after the last + * one. + */ + if (lnext(stmtlist_item) != NULL) + CommandCounterIncrement(); } /* Pop the snapshot if we pushed one. */