postgres.git / summary / log / commit / refs

commit    7f8356bd10b5b2a7dd092e158f5ffbf65e6a2c14
Author:   Michael Paquier <michael@paquier.xyz>
Date:     Wed Nov 14 07:48:10 2018 +0000
First released in: 12.0, 9.4.21, 9.5.16, 9.6.12, 10.7, 11.2

    Initialize TransactionState and user ID consistently at transaction start
    
    If a failure happens when a transaction is starting between the moment
    the transaction status is changed from TRANS_DEFAULT to TRANS_START and
    the moment the current user ID and security context flags are fetched
    via GetUserIdAndSecContext(), or before initializing its basic fields,
    then those may get reset to incorrect values when the transaction
    aborts, leaving the session in an inconsistent state.
    
    One problem reported is that failing a starting transaction at the first
    query of a session could cause several kinds of system crashes on the
    follow-up queries.
    
    In order to solve that, move the initialization of the transaction state
    fields and the call of GetUserIdAndSecContext() in charge of fetching
    the current user ID close to the point where the transaction status is
    switched to TRANS_START, where there cannot be any error triggered
    in-between, per an idea of Tom Lane.  This properly ensures that the
    current user ID, the security context flags and that the basic fields of
    TransactionState remain consistent even if the transaction fails while
    starting.
    
    Reported-by: Richard Guo
    Diagnosed-By: Richard Guo
    Author: Michael Paquier
    Reviewed-by: Tom Lane
    Discussion: https://postgr.es/m/CAN_9JTxECSb=pEPcb0a8d+6J+bDcOZ4=DgRo_B7Y5gRHJUM=Rw@mail.gmail.com
    Backpatch-through: 9.4


 src/backend/access/transam/xact.c | 48 +++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index b028b318e5c..74d95057469 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1834,20 +1834,38 @@ StartTransaction(void)
 
 	Assert(XactTopTransactionId == InvalidTransactionId);
 
-	/*
-	 * check the current transaction state
-	 */
-	if (s->state != TRANS_DEFAULT)
-		elog(WARNING, "StartTransaction while in %s state",
-			 TransStateAsString(s->state));
+	/* check the current transaction state */
+	Assert(s->state == TRANS_DEFAULT);
 
 	/*
-	 * set the current transaction state information appropriately during
-	 * start processing
+	 * Set the current transaction state information appropriately during
+	 * start processing.  Note that once the transaction status is switched
+	 * this process cannot fail until the user ID and the security context
+	 * flags are fetched below.
 	 */
 	s->state = TRANS_START;
 	s->transactionId = InvalidTransactionId;	/* until assigned */
 
+	/*
+	 * initialize current transaction state fields
+	 *
+	 * note: prevXactReadOnly is not used at the outermost level
+	 */
+	s->nestingLevel = 1;
+	s->gucNestLevel = 1;
+	s->childXids = NULL;
+	s->nChildXids = 0;
+	s->maxChildXids = 0;
+
+	/*
+	 * Once the current user ID and the security context flags are fetched,
+	 * both will be properly reset even if transaction startup fails.
+	 */
+	GetUserIdAndSecContext(&s->prevUser, &s->prevSecContext);
+
+	/* SecurityRestrictionContext should never be set outside a transaction */
+	Assert(s->prevSecContext == 0);
+
 	/*
 	 * Make sure we've reset xact state variables
 	 *
@@ -1928,20 +1946,6 @@ StartTransaction(void)
 	xactStopTimestamp = 0;
 	pgstat_report_xact_timestamp(xactStartTimestamp);
 
-	/*
-	 * initialize current transaction state fields
-	 *
-	 * note: prevXactReadOnly is not used at the outermost level
-	 */
-	s->nestingLevel = 1;
-	s->gucNestLevel = 1;
-	s->childXids = NULL;
-	s->nChildXids = 0;
-	s->maxChildXids = 0;
-	GetUserIdAndSecContext(&s->prevUser, &s->prevSecContext);
-	/* SecurityRestrictionContext should never be set outside a transaction */
-	Assert(s->prevSecContext == 0);
-
 	/*
 	 * initialize other subsystems for new transaction
 	 */


[parent: 32060f6780bc]