FinishPreparedTransaction missing HOLD_INTERRUPTS section
Hello.
It seems that during COMMIT PREPARED FinishPreparedTransaction() doesn't
hold interrupts around writing to wal and cleaning up ProcArray and GXact
entries. At least RemoveTwoPhaseFile (which is called in between) can print
a warning with ereport(), which, in turn will check for interrupts and
therefore can cancel backend or throw an error before GXact clean-up.
Other similar places like CommitTransaction and PrepareTransaction have
such hold interrupts sections.
--
Stas Kelvich
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachments:
0001-Add-HOLD_INTERRUPTS-section-into-FinishPreparedTrans.patchapplication/octet-stream; name=0001-Add-HOLD_INTERRUPTS-section-into-FinishPreparedTrans.patch; x-unix-mode=0644Download
From 8760cbda323b6e725db44e8c26d8d6a063d692a5 Mon Sep 17 00:00:00 2001
From: Stas Kelvich <stanconn@gmail.com>
Date: Fri, 27 Apr 2018 23:52:26 +0300
Subject: [PATCH] Add HOLD_INTERRUPTS section into FinishPreparedTransaction.
If an interrupt arrives in the middle of FinishPreparedTransaction
and any callback decide to call CHECK_FOR_INTERRUPTS (e.g.
RemoveTwoPhaseFile can write a warning with ereport, which checks for
interrupts) then it's possible to leave current GXact undeleted.
---
src/backend/access/transam/twophase.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index cdd8156ce4..a68769fb0d 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1487,6 +1487,9 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
/* compute latestXid among all children */
latestXid = TransactionIdLatest(xid, hdr->nsubxacts, children);
+ /* Prevent cancel/die interrupt while cleaning up */
+ HOLD_INTERRUPTS();
+
/*
* The order of operations here is critical: make the XLOG entry for
* commit or abort, then mark the transaction committed or aborted in
@@ -1579,6 +1582,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
MyLockedGxact = NULL;
pfree(buf);
+ RESUME_INTERRUPTS();
}
/*
--
2.16.2
On Sat, Apr 28, 2018 at 12:36:16AM +0300, Stas Kelvich wrote:
It seems that during COMMIT PREPARED FinishPreparedTransaction() doesn't
hold interrupts around writing to wal and cleaning up ProcArray and GXact
entries. At least RemoveTwoPhaseFile (which is called in between) can print
a warning with ereport(), which, in turn will check for interrupts and
therefore can cancel backend or throw an error before GXact clean-up.Other similar places like CommitTransaction and PrepareTransaction have
such hold interrupts sections.
Good catch! The places you are suggesting look good to me as well.
That's something which should be back-patched as well.
--
Michael
Thank you, pushed!
Stas Kelvich wrote:
Hello.
It seems that during COMMIT PREPARED FinishPreparedTransaction() doesn't
hold interrupts around writing to wal and cleaning up ProcArray and GXact
entries. At least RemoveTwoPhaseFile (which is called in between) can print
a warning with ereport(), which, in turn will check for interrupts and
therefore can cancel backend or throw an error before GXact clean-up.Other similar places like CommitTransaction and PrepareTransaction have
such hold interrupts sections.--
Stas Kelvich
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
--
Teodor Sigaev E-mail: teodor@sigaev.ru
WWW: http://www.sigaev.ru/