diff options
| -rw-r--r-- | src/backend/commands/portalcmds.c | 8 | ||||
| -rw-r--r-- | src/backend/tcop/pquery.c | 6 | ||||
| -rw-r--r-- | src/backend/utils/mmgr/portalmem.c | 53 | ||||
| -rw-r--r-- | src/include/utils/portal.h | 1 | ||||
| -rw-r--r-- | src/test/regress/expected/transactions.out | 33 | ||||
| -rw-r--r-- | src/test/regress/sql/transactions.sql | 13 |
6 files changed, 72 insertions, 42 deletions
diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c index 89086aa3717..656a61cbdeb 100644 --- a/src/backend/commands/portalcmds.c +++ b/src/backend/commands/portalcmds.c @@ -224,7 +224,7 @@ PerformPortalClose(const char *name) } /* - * Note: PortalCleanup is called as a side-effect + * Note: PortalCleanup is called as a side-effect, if not already done. */ PortalDrop(portal, false); } @@ -234,6 +234,10 @@ PerformPortalClose(const char *name) * * Clean up a portal when it's dropped. This is the standard cleanup hook * for portals. + * + * Note: if portal->status is PORTAL_FAILED, we are probably being called + * during error abort, and must be careful to avoid doing anything that + * is likely to fail again. */ void PortalCleanup(Portal portal) @@ -420,7 +424,7 @@ PersistHoldablePortal(Portal portal) PG_CATCH(); { /* Uncaught error while executing portal: mark it dead */ - portal->status = PORTAL_FAILED; + MarkPortalFailed(portal); /* Restore global vars and propagate error */ ActivePortal = saveActivePortal; diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index b7649c68fcf..0e2494fa8a5 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -608,7 +608,7 @@ PortalStart(Portal portal, ParamListInfo params, Snapshot snapshot) PG_CATCH(); { /* Uncaught error while executing portal: mark it dead */ - portal->status = PORTAL_FAILED; + MarkPortalFailed(portal); /* Restore global vars and propagate error */ ActivePortal = saveActivePortal; @@ -830,7 +830,7 @@ PortalRun(Portal portal, long count, bool isTopLevel, PG_CATCH(); { /* Uncaught error while executing portal: mark it dead */ - portal->status = PORTAL_FAILED; + MarkPortalFailed(portal); /* Restore global vars and propagate error */ if (saveMemoryContext == saveTopTransactionContext) @@ -1447,7 +1447,7 @@ PortalRunFetch(Portal portal, PG_CATCH(); { /* Uncaught error while executing portal: mark it dead */ - portal->status = PORTAL_FAILED; + MarkPortalFailed(portal); /* Restore global vars and propagate error */ ActivePortal = saveActivePortal; diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c index 186548dcba5..9a257e7351b 100644 --- a/src/backend/utils/mmgr/portalmem.c +++ b/src/backend/utils/mmgr/portalmem.c @@ -404,6 +404,8 @@ UnpinPortal(Portal portal) /* * MarkPortalDone * Transition a portal from ACTIVE to DONE state. + * + * NOTE: never set portal->status = PORTAL_DONE directly; call this instead. */ void MarkPortalDone(Portal portal) @@ -417,8 +419,36 @@ MarkPortalDone(Portal portal) * well do that now, since the portal can't be executed any more. * * In some cases involving execution of a ROLLBACK command in an already - * aborted transaction, this prevents an assertion failure from reaching - * AtCleanup_Portals with the cleanup hook still unexecuted. + * aborted transaction, this prevents an assertion failure caused by + * reaching AtCleanup_Portals with the cleanup hook still unexecuted. + */ + if (PointerIsValid(portal->cleanup)) + { + (*portal->cleanup) (portal); + portal->cleanup = NULL; + } +} + +/* + * MarkPortalFailed + * Transition a portal into FAILED state. + * + * NOTE: never set portal->status = PORTAL_FAILED directly; call this instead. + */ +void +MarkPortalFailed(Portal portal) +{ + /* Perform the state transition */ + Assert(portal->status != PORTAL_DONE); + portal->status = PORTAL_FAILED; + + /* + * Allow portalcmds.c to clean up the state it knows about. We might as + * well do that now, since the portal can't be executed any more. + * + * In some cases involving cleanup of an already aborted transaction, this + * prevents an assertion failure caused by reaching AtCleanup_Portals with + * the cleanup hook still unexecuted. */ if (PointerIsValid(portal->cleanup)) { @@ -454,6 +484,9 @@ PortalDrop(Portal portal, bool isTopCommit) * hook's responsibility to not try to do that more than once, in the case * that failure occurs and then we come back to drop the portal again * during transaction abort. + * + * Note: in most paths of control, this will have been done already in + * MarkPortalDone or MarkPortalFailed. We're just making sure. */ if (PointerIsValid(portal->cleanup)) { @@ -712,7 +745,7 @@ AtAbort_Portals(void) /* Any portal that was actually running has to be considered broken */ if (portal->status == PORTAL_ACTIVE) - portal->status = PORTAL_FAILED; + MarkPortalFailed(portal); /* * Do nothing else to cursors held over from a previous transaction. @@ -727,9 +760,12 @@ AtAbort_Portals(void) * AtSubAbort_Portals. */ if (portal->status == PORTAL_READY) - portal->status = PORTAL_FAILED; + MarkPortalFailed(portal); - /* let portalcmds.c clean up the state it knows about */ + /* + * Allow portalcmds.c to clean up the state it knows about, if we + * haven't already. + */ if (PointerIsValid(portal->cleanup)) { (*portal->cleanup) (portal); @@ -861,9 +897,12 @@ AtSubAbort_Portals(SubTransactionId mySubid, */ if (portal->status == PORTAL_READY || portal->status == PORTAL_ACTIVE) - portal->status = PORTAL_FAILED; + MarkPortalFailed(portal); - /* let portalcmds.c clean up the state it knows about */ + /* + * Allow portalcmds.c to clean up the state it knows about, if we + * haven't already. + */ if (PointerIsValid(portal->cleanup)) { (*portal->cleanup) (portal); diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h index 6af1cd5b106..cf50655e125 100644 --- a/src/include/utils/portal.h +++ b/src/include/utils/portal.h @@ -207,6 +207,7 @@ extern Portal CreateNewPortal(void); extern void PinPortal(Portal portal); extern void UnpinPortal(Portal portal); extern void MarkPortalDone(Portal portal); +extern void MarkPortalFailed(Portal portal); extern void PortalDrop(Portal portal, bool isTopCommit); extern Portal GetPortalByName(const char *name); extern void PortalDefineQuery(Portal portal, diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out index f49ec0effee..e9d3908b1ab 100644 --- a/src/test/regress/expected/transactions.out +++ b/src/test/regress/expected/transactions.out @@ -601,28 +601,11 @@ fetch from foo; (1 row) abort; --- tests for the "tid" type -SELECT '(3, 3)'::tid = '(3, 4)'::tid; - ?column? ----------- - f -(1 row) - -SELECT '(3, 3)'::tid = '(3, 3)'::tid; - ?column? ----------- - t -(1 row) - -SELECT '(3, 3)'::tid <> '(3, 3)'::tid; - ?column? ----------- - f -(1 row) - -SELECT '(3, 3)'::tid <> '(3, 4)'::tid; - ?column? ----------- - t -(1 row) - +-- Test for successful cleanup of an aborted transaction at session exit. +-- THIS MUST BE THE LAST TEST IN THIS FILE. +begin; +select 1/0; +ERROR: division by zero +rollback to X; +ERROR: no such savepoint +-- DO NOT ADD ANYTHING HERE. diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql index 23271c8eaba..faf6a9bf938 100644 --- a/src/test/regress/sql/transactions.sql +++ b/src/test/regress/sql/transactions.sql @@ -368,8 +368,11 @@ fetch from foo; abort; --- tests for the "tid" type -SELECT '(3, 3)'::tid = '(3, 4)'::tid; -SELECT '(3, 3)'::tid = '(3, 3)'::tid; -SELECT '(3, 3)'::tid <> '(3, 3)'::tid; -SELECT '(3, 3)'::tid <> '(3, 4)'::tid; +-- Test for successful cleanup of an aborted transaction at session exit. +-- THIS MUST BE THE LAST TEST IN THIS FILE. + +begin; +select 1/0; +rollback to X; + +-- DO NOT ADD ANYTHING HERE. |