From: Stas Kelvich <s.kelvich@postgrespro.ru>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_2F0ED1C6-4FAA-48EA-BD6D-1DE47DDF32B6"
Mime-Version: 1.0 (Mac OS X Mail 11.2 \(3445.5.20\))
Subject: FinishPreparedTransaction missing HOLD_INTERRUPTS section
Message-Id: <3AD85097-A3F3-4EBA-99BD-C38EDF8D2949@postgrespro.ru>
Date: Sat, 28 Apr 2018 00:36:16 +0300
To: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Precedence: bulk


--Apple-Mail=_2F0ED1C6-4FAA-48EA-BD6D-1DE47DDF32B6
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
	charset=us-ascii

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


--Apple-Mail=_2F0ED1C6-4FAA-48EA-BD6D-1DE47DDF32B6
Content-Disposition: attachment;
	filename=0001-Add-HOLD_INTERRUPTS-section-into-FinishPreparedTrans.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="0001-Add-HOLD_INTERRUPTS-section-into-FinishPreparedTrans.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=208760cbda323b6e725db44e8c26d8d6a063d692a5=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20Stas=20Kelvich=20<stanconn@gmail.com>=0ADate:=20=
Fri,=2027=20Apr=202018=2023:52:26=20+0300=0ASubject:=20[PATCH]=20Add=20=
HOLD_INTERRUPTS=20section=20into=20FinishPreparedTransaction.=0A=0AIf=20=
an=20interrupt=20arrives=20in=20the=20middle=20of=20=
FinishPreparedTransaction=0Aand=20any=20callback=20decide=20to=20call=20=
CHECK_FOR_INTERRUPTS=20(e.g.=0ARemoveTwoPhaseFile=20can=20write=20a=20=
warning=20with=20ereport,=20which=20checks=20for=0Ainterrupts)=20then=20=
it's=20possible=20to=20leave=20current=20GXact=20undeleted.=0A---=0A=20=
src/backend/access/transam/twophase.c=20|=204=20++++=0A=201=20file=20=
changed,=204=20insertions(+)=0A=0Adiff=20--git=20=
a/src/backend/access/transam/twophase.c=20=
b/src/backend/access/transam/twophase.c=0Aindex=20cdd8156ce4..a68769fb0d=20=
100644=0A---=20a/src/backend/access/transam/twophase.c=0A+++=20=
b/src/backend/access/transam/twophase.c=0A@@=20-1487,6=20+1487,9=20@@=20=
FinishPreparedTransaction(const=20char=20*gid,=20bool=20isCommit)=0A=20=09=
/*=20compute=20latestXid=20among=20all=20children=20*/=0A=20=09latestXid=20=
=3D=20TransactionIdLatest(xid,=20hdr->nsubxacts,=20children);=0A=20=0A+=09=
/*=20Prevent=20cancel/die=20interrupt=20while=20cleaning=20up=20*/=0A+=09=
HOLD_INTERRUPTS();=0A+=0A=20=09/*=0A=20=09=20*=20The=20order=20of=20=
operations=20here=20is=20critical:=20make=20the=20XLOG=20entry=20for=0A=20=
=09=20*=20commit=20or=20abort,=20then=20mark=20the=20transaction=20=
committed=20or=20aborted=20in=0A@@=20-1579,6=20+1582,7=20@@=20=
FinishPreparedTransaction(const=20char=20*gid,=20bool=20isCommit)=0A=20=09=
MyLockedGxact=20=3D=20NULL;=0A=20=0A=20=09pfree(buf);=0A+=09=
RESUME_INTERRUPTS();=0A=20}=0A=20=0A=20/*=0A--=20=0A2.16.2=0A=0A=

--Apple-Mail=_2F0ED1C6-4FAA-48EA-BD6D-1DE47DDF32B6--