From ce95d0f14d2a6374014e852a6a1f20d30a0f69f6 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 24 Jan 2010 21:50:09 +0000 Subject: Fix assorted core dumps and Assert failures that could occur during AbortTransaction or AbortSubTransaction, when trying to clean up after an error that prevented (sub)transaction start from completing: * access to TopTransactionResourceOwner that might not exist * assert failure in AtEOXact_GUC, if AtStart_GUC not called yet * assert failure or core dump in AfterTriggerEndSubXact, if AfterTriggerBeginSubXact not called yet Per testing by injecting elog(ERROR) at successive steps in StartTransaction and StartSubTransaction. It's not clear whether all of these cases could really occur in the field, but at least one of them is easily exposed by simple stress testing, as per my accidental discovery yesterday. --- src/backend/access/transam/xact.c | 53 +++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 25 deletions(-) (limited to 'src/backend/access/transam/xact.c') diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index b2e32d699fb..d032c411db7 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.195.4.5 2009/12/09 21:58:54 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.195.4.6 2010/01/24 21:50:06 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1699,32 +1699,35 @@ AbortTransaction(void) /* * Post-abort cleanup. See notes in CommitTransaction() concerning - * ordering. + * ordering. We can skip all of it if the transaction failed before + * creating a resource owner. */ + if (TopTransactionResourceOwner != NULL) + { + CallXactCallbacks(XACT_EVENT_ABORT); - CallXactCallbacks(XACT_EVENT_ABORT); - - ResourceOwnerRelease(TopTransactionResourceOwner, - RESOURCE_RELEASE_BEFORE_LOCKS, - false, true); - AtEOXact_Buffers(false); - AtEOXact_Inval(false); - smgrDoPendingDeletes(false); - ResourceOwnerRelease(TopTransactionResourceOwner, - RESOURCE_RELEASE_LOCKS, - false, true); - ResourceOwnerRelease(TopTransactionResourceOwner, - RESOURCE_RELEASE_AFTER_LOCKS, - false, true); - - AtEOXact_GUC(false, 1); - AtEOXact_SPI(false); - AtEOXact_on_commit_actions(false); - AtEOXact_Namespace(false); - smgrabort(); - AtEOXact_Files(); - AtEOXact_HashTables(false); - pgstat_count_xact_rollback(); + ResourceOwnerRelease(TopTransactionResourceOwner, + RESOURCE_RELEASE_BEFORE_LOCKS, + false, true); + AtEOXact_Buffers(false); + AtEOXact_Inval(false); + smgrDoPendingDeletes(false); + ResourceOwnerRelease(TopTransactionResourceOwner, + RESOURCE_RELEASE_LOCKS, + false, true); + ResourceOwnerRelease(TopTransactionResourceOwner, + RESOURCE_RELEASE_AFTER_LOCKS, + false, true); + + AtEOXact_GUC(false, 1); + AtEOXact_SPI(false); + AtEOXact_on_commit_actions(false); + AtEOXact_Namespace(false); + smgrabort(); + AtEOXact_Files(); + AtEOXact_HashTables(false); + pgstat_count_xact_rollback(); + } /* * State remains TRANS_ABORT until CleanupTransaction(). -- cgit v1.2.3