Feedback-ID: i0fe9450f:Fastmail
Date: Tue, 19 Mar 2024 12:57:51 +0900
From: Michael Paquier <michael@paquier.xyz>
To: feichanghong <feichanghong@qq.com>
Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>,
	andres <andres@anarazel.de>, "sawada.mshk" <sawada.mshk@gmail.com>,
	"horikyota.ntt" <horikyota.ntt@gmail.com>
Subject: Re: ReplicationSlotRelease may set the statusFlags of other
 processes in PG14
Message-ID: <ZfkNP1OdgBSPPTsR@paquier.xyz>
References: <tencent_CA7420C82971BC4B64F0748A7D2898A4720A@qq.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="mHRSu8Ntt/AHoW6h"
Content-Disposition: inline
In-Reply-To: <tencent_CA7420C82971BC4B64F0748A7D2898A4720A@qq.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ZfkNP1OdgBSPPTsR%40paquier.xyz>
Precedence: bulk


--mHRSu8Ntt/AHoW6h
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Mar 16, 2024 at 10:29:03PM +0800, feichanghong wrote:
> A process utilizing replication slots (usually walsender) calls callback
> functions in the order of RemoveProcFromArray->ProcKill upon abnormal exi=
t.
> Within RemoveProcFromArray, MyProc is already removed from the ProcArray.
> ProcKill then attempts to set ProcGlobal->statusFlags[MyProc->pgxactoff] =
again
> via ReplicationSlotRelease. By this time, the flag may already be assigne=
d to
> another process.

Oops.

> To replicate the issue, execute the following steps:
> 1. Apply the attached v1-0000-v14-invalidate-pgxactoff-after-remove-pgpro=
c.patch,
> where pgxactoff is set to an invalid value in ProcArrayRemove, and some
> checks are added.
> 2. Use the SQL below to terminate the walsender process.
> ```
> select pg_terminate_backend(pid) from pg_stat_activity where backend_type=
 =3D 'walsender';
> ```
> # Fix
>=20
> To fix the issue, I have provided some patches in the attachment:
> 1. Backpatching 2f6501f into the PG14 version will fix the problem.
> 2. In PG14-head, ProcArrayRemove needs to reset pgxactoff, and some assert
> checks should be done when setting ProcGlobal->statusFlags.

Yeah, that's something that we had better fix in all stable branches.
The asserts would offer some protection moving on, but I would take
the safer move of only adding a protection like what you are
suggestion on HEAD and not in stable branches, just in case we're
missing something around them.
--
Michael

--mHRSu8Ntt/AHoW6h
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmX5DT8ACgkQnvQgOdby
QH2GHw//ekSskK8zXIiMoOnmhWnlhO85QrzJ920T48E3q2JzidTKNERqsanR3Q6q
JCZiZYqjrCi6eKJabhCnqcarvAvSMCf0fbvSAZHpccPl2gbIqNLcsZwL0xW8eRIV
+J8ihKh3x1n3vcn18ZKoAxqiIY+IfZcVNrLH+lmYM5RIHjSbgXq2NqNBg1mcFA15
PMYgLg1rTVyYBst9rEwvCvg2CwFMXU5wcYSMOMktTGh9djAfh258Lg/ox6X9iUJK
JVUWa5/AIdonBNHdxA8GoB9XwV7p7Cfep+YGCyUGwRPiMbNJ3Ppaup+6wD0r7qC9
auvY0NPXar500GokER+qsvkIMnnkPkZo0Ep6wYUNgaVcjYB/etECJ7SvqOEYDEze
gQbFPdfO+pevwbaZQYz3yaXPKfKY4xvE7tve/cVUfit/y6GaTin6l6ta8V2vbT8E
FWPt6fkOPo7uSC+701fIUL8DH07lH2Hs2iWgV6VkqKY7YuMumAnhRyD50bJn79Xc
seoGNrBm+PEtYHkkToel8fcLeDGDP0elt/uwIV38t99SzcRGhkQ6K0BU9giWC68i
ZAu05FFdHISWLM5+egHZ70m+Ou7OJ1u26fL63GeDro8ePDtRr6Rg/DEv3dyFYiuW
TxIWsUagUnziT8P862b5RR1YKdlAzWsEjyWDQgRInPPNYdFEtuA=
=d64x
-----END PGP SIGNATURE-----

--mHRSu8Ntt/AHoW6h--