8000 Fix race condition in preparing a transaction for two-phase commit. · larkly/postgres-docker@8c19b80 · GitHub
[go: up one dir, main page]

Skip to content

Commit 8c19b80

Browse files
committed
Fix race condition in preparing a transaction for two-phase commit.
To lock a prepared transaction's shared memory entry, we used to mark it with the XID of the backend. When the XID was no longer active according to the proc array, the entry was implicitly considered as not locked anymore. However, when preparing a transaction, the backend's proc array entry was cleared before transfering the locks (and some other state) to the prepared transaction's dummy PGPROC entry, so there was a window where another backend could finish the transaction before it was in fact fully prepared. To fix, rewrite the locking mechanism of global transaction entries. Instead of an XID, just have simple locked-or-not flag in each entry (we store the locking backend's backend id rather than a simple boolean, but that's just for debugging purposes). The backend is responsible for explicitly unlocking the entry, and to make sure that that happens, install a callback to unlock it on abort or process exit. Backpatch to all supported versions.
1 parent 360ec00 commit 8c19b80

File tree

3 files changed

+144
-47
lines changed

3 files changed

+144
-47
lines changed

src/backend/access/transam/twophase.c

Lines changed: 125 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
#include "replication/walsender.h"
5959
#include "replication/syncrep.h"
6060
#include "storage/fd.h"
61+
#include "storage/ipc.h"
6162
#include "storage/predicate.h"
6263
#include "storage/procarray.h"
6364
#include "storage/sinvaladt.h"
@@ -85,25 +86,25 @@ int max_prepared_xacts = 0;
8586
*
8687
* The lifecycle of a global transaction is:
8788
*
88-
* 1. After checking that the requested GID is not in use, set up an
89-
* entry in the TwoPhaseState->prepXacts array with the correct XID and GID,
90-
* with locking_xid = my own XID and valid = false.
89+
* 1. After checking that the requested GID is not in use, set up an entry in
90+
* the TwoPhaseState->prepXacts array with the correct GID and valid = false,
91+
* and mark it as locked by my backend.
9192
*
9293
* 2. After successfully completing prepare, set valid = true and enter the
9394
* contained PGPROC into the global ProcArray.
9495
*
95-
* 3. To begin COMMIT PREPARED or ROLLBACK PREPARED, check that the entry
96-
* is valid and its locking_xid is no longer active, then store my current
97-
* XID into locking_xid. This prevents concurrent attempts to commit or
98-
* rollback the same prepared xact.
96+
* 3. To begin COMMIT PREPARED or ROLLBACK PREPARED, check that the entry is
97+
* valid and not locked, then mark the entry as locked by storing my current
98+
* backend ID into locking_backend. This prevents concurrent attempts to
99+
* commit or rollback the same prepared xact.
99100
*
100101
* 4. On completion of COMMIT PREPARED or ROLLBACK PREPARED, remove the entry
101102
* from the ProcArray and the TwoPhaseState->prepXacts array and return it to
102103
* the freelist.
103104
*
104105
* Note that if the preparing transaction fails between steps 1 and 2, the
105-
* entry will remain in prepXacts until recycled. We can detect recyclable
106-
* entries by checking for valid = false and locking_xid no longer active.
106+
* entry must be removed so that the GID and the GlobalTransaction struct
107+
* can be reused. See AtAbort_Twophase().
107108
*
108109
* typedef struct GlobalTransactionData *GlobalTransaction appears in
109110
* twophase.h
@@ -117,8 +118,8 @@ typedef struct GlobalTransactionData
117< 8000 code>118
TimestampTz prepared_at; /* time of preparation */
118119
XLogRecPtr prepare_lsn; /* XLOG offset of prepare record */
119120
Oid owner; /* ID of user that executed the xact */
120-
TransactionId locking_xid; /* top-level XID of backend working on xact */
121-
bool valid; /* TRUE if fully prepared */
121+
BackendId locking_backend; /* backend currently working on the xact */
122+
bool valid; /* TRUE if PGPROC entry is in proc array */
122123
char gid[GIDSIZE]; /* The GID assigned to the prepared xact */
123124
} GlobalTransactionData;
124125

@@ -143,6 +144,12 @@ typedef struct TwoPhaseStateData
143144

144145
static TwoPhaseStateData *TwoPhaseState;
145146

147+
/*
148+
* Global transaction entry currently locked by us, if any.
149+
*/
150+
static GlobalTransaction MyLockedGxact = NULL;
151+
152+
static bool twophaseExitRegistered = false;
146153

147154
static void RecordTransactionCommitPrepared(TransactionId xid,
148155
int nchildren,
@@ -159,6 +166,7 @@ static void RecordTransactionAbortPrepared(TransactionId xid,
159166
RelFileNode *rels);
160167
static void ProcessRecords(char *bufptr, TransactionId xid,
161168
const TwoPhaseCallback callbacks[]);
169+
static void RemoveGXact(GlobalTransaction gxact);
162170

163171

164172
/*
@@ -228,6 +236,74 @@ TwoPhaseShmemInit(void)
228236
Assert(found);
229237
}
230238

239+
/*
240+
* Exit hook to unlock the global transaction entry we're working on.
241+
*/
242+
static void
243+
AtProcExit_Twophase(int code, Datum arg)
244+
{
245+
/* same logic as abort */
246+
AtAbort_Twophase();
247+
}
248+
249+
/*
250+
* Abort hook to unlock the global transaction entry we're working on.
251+
*/
252+
void
253+
AtAbort_Twophase(void)
254+
{
255+
if (MyLockedGxact == NULL)
256+
return;
257+
258+
/*
259+
* What to do with the locked global transaction entry? If we were in
260+
* the process of preparing the transaction, but haven't written the WAL
261+
* record and state file yet, the transaction must not be considered as
262+
* prepared. Likewise, if we are in the process of finishing an
263+
* already-prepared transaction, and fail after having already written
264+
* the 2nd phase commit or rollback record to the WAL, the transaction
265+
* should not be considered as prepared anymore. In those cases, just
266+
* remove the entry from shared memory.
267+
*
268+
* Otherwise, the entry must be left in place so that the transaction
269+
* can be finished later, so just unlock it.
270+
*
271+
* If we abort during prepare, after having written the WAL record, we
272+
* might not have transfered all locks and other state to the prepared
273+
* transaction yet. Likewise, if we abort during commit or rollback,
274+
* after having written the WAL record, we might not have released
275+
* all the resources held by the transaction yet. In those cases, the
276+
* in-memory state can be wrong, but it's too late to back out.
277+
*/
278+
if (!MyLockedGxact->valid)
279+
{
280+
RemoveGXact(MyLockedGxact);
281+
}
282+
else
283+
{
284+
LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE);
285+
286+
MyLockedGxact->locking_backend = InvalidBackendId;
287+
288+
LWLockRelease(TwoPhaseStateLock);
289+
}
290+
MyLockedGxact = NULL;
291+
}
292+
293+
/*
294+
* This is called after we have finished transfering state to the prepared
295+
* PGXACT entry.
296+
*/
297+
void
298+
PostPrepare_Twophase()
299+
{
300+
LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE);
301+
MyLockedGxact->locking_backend = InvalidBackendId;
302+
LWLockRelease(TwoPhaseStateLock);
303+
304+
MyLockedGxact = NULL;
305+
}
306+
231307

232308
/*
233309
* MarkAsPreparing
@@ -257,29 +333,15 @@ MarkAsPreparing(TransactionId xid, const char *gid,
257333
errmsg("prepared transactions are disabled"),
258334
errhint("Set max_prepared_transactions to a nonzero value.")));
259335

260-
LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE);
261-
262-
/*
263-
* First, find and recycle any gxacts that failed during prepare. We do
264-
* this partly to ensure we don't mistakenly say their GIDs are still
265-
* reserved, and partly so we don't fail on out-of-slots unnecessarily.
266-
*/
267-
for (i = 0; i < TwoPhaseState->numPrepXacts; i++)
336+
/* on first call, register the exit hook */
337+
if (!twophaseExitRegistered)
268338
{
269-
gxact = TwoPhaseState->prepXacts[i];
270-
if (!gxact->valid && !TransactionIdIsActive(gxact->locking_xid))
271-
{
272-
/* It's dead Jim ... remove from the active array */
273-
TwoPhaseState->numPrepXacts--;
274-
TwoPhaseState->prepXacts[i] = TwoPhaseState->prepXacts[TwoPhaseState->numPrepXacts];
275-
/* and put it back in the freelist */
276-
gxact->proc.links.next = (SHM_QUEUE *) TwoPhaseState->freeGXacts;
277-
TwoPhaseState->freeGXacts = gxact;
278-
/* Back up index count too, so we don't miss scanning one */
279-
i--;
280-
}
339+
on_shmem_exit(AtProcExit_Twophase, 0);
340+
twophaseExitRegistered = true;
281341
}
282342

343+
LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE);
344+
283345
/* Check for conflicting GID */
284346
for (i = 0; i < TwoPhaseState->numPrepXacts; i++)
285347
{
@@ -333,14 +395,20 @@ MarkAsPreparing(TransactionId xid, const char *gid,
333395
gxact->prepare_lsn.xlogid = 0;
334396
gxact->prepare_lsn.xrecoff = 0;
335397
gxact->owner = owner;
336-
gxact->locking_xid = xid;
398+
gxact->locking_backend = MyBackendId;
337399
gxact->valid = false;
338400
strcpy(gxact->gid, gid);
339401

340402
/* And insert it into the active array */
341403
Assert(TwoPhaseState->numPrepXacts < max_prepared_xacts);
342404
TwoPhaseState->prepXacts[TwoPhaseState->numPrepXacts++] = gxact;
343405

406+
/*
407+
* Remember that we have this GlobalTransaction entry locked for us.
408+
* If we abort after this, we must release it.
409+
*/
410+
MyLockedGxact = gxact;
411+
344412
LWLockRelease(TwoPhaseStateLock);
345413

346414
return gxact;
@@ -400,6 +468,13 @@ LockGXact(const char *gid, Oid user)
400468
{
401469
int i;
402470

471+
/* on first call, register the exit hook */
472+
if (!twophaseExitRegistered)
473+
{
474+
on_shmem_exit(AtProcExit_Twophase, 0);
475+
twophaseExitRegistered = true;
476+
}
477+
403478
LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE);
404479

405480
for (i = 0; i < TwoPhaseState->numPrepXacts; i++)
@@ -413,15 +488,11 @@ LockGXact(const char *gid, Oid user)
413488
continue;
414489

415490
/* Found it, but has someone else got it locked? */
416-
if (TransactionIdIsValid(gxact->locking_xid))
417-
{
418-
if (TransactionIdIsActive(gxact->locking_xid))
419-
ereport(ERROR,
420-
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
421-
errmsg("prepared transaction with identifier \"%s\" is busy",
422-
gid)));
423-
gxact->locking_xid = InvalidTransactionId;
424-
}
491+
if (gxact->locking_backend != InvalidBackendId)
492+
ereport(ERROR,
493+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
494+
errmsg("prepared transaction with identifier \"%s\" is busy",
495+
gid)));
425496

426497
if (user != gxact->owner && !superuser_arg(user))
427498
ereport(ERROR,
@@ -442,7 +513,8 @@ LockGXact(const char *gid, Oid user)
442513
errhint("Connect to the database where the transaction was prepared to finish it.")));
443514

444515
/* OK for me to lock it */
445-
gxact->locking_xid = GetTopTransactionId();
516+
gxact->locking_backend = MyBackendId;
517+
MyLockedGxact = gxact;
446518

447519
LWLockRelease(TwoPhaseStateLock);
448520

@@ -1070,6 +1142,13 @@ EndPrepare(GlobalTransaction gxact)
10701142
*/
10711143
MyProc->inCommit = false;
10721144

1145+
/*
1146+
* Remember that we have this GlobalTransaction entry locked for us. If
1147+
* we crash after this point, it's too late to abort, but we must unlock
1148+
* it so that the prepared transaction can be committed or rolled back.
1149+
*/
1150+
MyLockedGxact = gxact;
1151+
10731152
END_CRIT_SECTION();
10741153

10751154
/*
@@ -1312,8 +1391,9 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
13121391

13131392
/*
13141393
* In case we fail while running the callbacks, mark the gxact invalid so
1315-
* no one else will try to commit/rollback, and so it can be recycled
1316-
* properly later. It is still locked by our XID so it won't go away yet.
1394+
* no one else will try to commit/rollback, and so it will be recycled
1395+
* if we fail after this point. It is still locked by our backend so it
1396+
* won't go away yet.
13171397
*
13181398
* (We assume it's safe to do this without taking TwoPhaseStateLock.)
13191399
*/
@@ -1377,6 +1457,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
13771457
RemoveTwoPhaseFile(xid, true);
13781458

13791459
RemoveGXact(gxact);
1460+
MyLockedGxact = NULL;
13801461

13811462
pfree(buf);
13821463
}

src/backend/access/transam/xact.c

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2115,9 +2115,13 @@ PrepareTransaction(void)
21152115
ProcArrayClearTransaction(MyProc);
21162116

21172117
/*
2118-
* This is all post-transaction cleanup. Note that if an error is raised
2119-
* here, it's too late to abort the transaction. This should be just
2120-
* noncritical resource releasing. See notes in CommitTransaction.
2118+
* In normal commit-processing, this is all non-critical post-transaction
2119+
* cleanup. When the transaction is prepared, however, it's important that
2120+
* the locks and other per-backend resources are transfered to the
2121+
* prepared transaction's PGPROC entry. Note that if an error is raised
2122+
* here, it's too late to abort the transaction. XXX: This probably should
2123+
* be in a critical section, to force a PANIC if any of this fails, but
2124+
* that cure could be worse than the disease.
21212125
*/
21222126

21232127
CallXactCallbacks(XACT_EVENT_PREPARE);
@@ -2155,6 +2159,14 @@ PrepareTransaction(void)
21552159
RESOURCE_RELEASE_AFTER_LOCKS,
21562160
true, true);
21572161

2162+
/*
2163+
* Allow another backend to finish the transaction. After
2164+
* PostPrepare_Twophase(), the transaction is completely detached from
2165+
* our backend. The rest is just non-critical cleanup of backend-local
2166+
* state.
2167+
*/
2168+
PostPrepare_Twophase();
2169+
21582170
/* Check we've released all catcache entries */
21592171
AtEOXact_CatCache(true);
21602172

@@ -2265,6 +2277,7 @@ AbortTransaction(void)
22652277
AtEOXact_LargeObject(false);
22662278
AtAbort_Notify();
22672279
AtEOXact_RelationMap(false);
2280+
AtAbort_Twophase();
22682281

22692282
/*
22702283
* Advertise the fact that we aborted in pg_clog (assuming that we got as

src/include/access/twophase.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ extern int max_prepared_xacts;
3131
extern Size TwoPhaseShmemSize(void);
3232
extern void TwoPhaseShmemInit(void);
3333

34+
extern void AtAbort_Twophase(void);
35+
extern void PostPrepare_Twophase(void);
36+
3437
extern PGPROC *TwoPhaseGetDummyProc(TransactionId xid);
3538
extern BackendId TwoPhaseGetDummyBackendId(TransactionId xid);
3639

0 commit comments

Comments
 (0)
0