Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 22 Jul 2026 10:29:23 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Zsolt Parragi <zsolt.parragi@percona.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: injection_points: canceled or terminated waiters leak their wait
 slots
Message-ID: <amAc853flInQAEI5@paquier.xyz>
References: 
 <CAN4CZFO+KF=cc0-iEg28RhqRBp_fTs6D4b8b7D7DB-pGYP3Ccg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="9AxgHkP5UHQ+qFd2"
Content-Disposition: inline
In-Reply-To: 
 <CAN4CZFO+KF=cc0-iEg28RhqRBp_fTs6D4b8b7D7DB-pGYP3Ccg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/amAc853flInQAEI5%40paquier.xyz>
Precedence: bulk

--9AxgHkP5UHQ+qFd2
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Jul 21, 2026 at 09:28:22AM +0100, Zsolt Parragi wrote:
> While stress-testing REPACK CONCURRENTLY on 19beta2 I saw a logical
> decoding activation race, and I extended 051_effective_wal_level.pl
> with a test that waits on an injection point and wakes it up later.
> 051 cancels two injection point waiters earlier in the script, and
> injection_wait() never cleans up after a canceled waiter, the wakeup
> never arrived and the test deadlocked.
>=20
> I think we are missing an ENSURE_ERROR_CLEANUP block there. See
> attached patch with a testcase reproducing the issue.

Hmm.  I think that I'd rather use a PG_TRY/PG_FINALLY and avoid the
refactoring with the extra routine required, keeping the cleanup
action local to injection_wait().  That's also because the cleanup
action is the same for both the "normal" exit path and the interrupt
path.

> I also attached a separate version for pg19, as master has a
> refactored version of injection_wait. All previous branches have the
> 19 version, it should be easy to backport to other branches.

Thanks for that.  I'm always OK to deal with a backpatch as required.
Posting versions saves some time, of course, just don't feel obliged
if you feel that this is extra work on your side.

On an unpatched code, the test would hang due to the fact that we are
doing a wait but we should not because the slot was not cleaned up.
It means that a failure mode equals to a timeout.  Why not, we have
other tests of this class.  Another thought: the addition of a SQL
function that provides the list of waiters that we reuse here.  I
don't really see why this is worth the cost compared to your test, but
opinions of others are welcome, of course.
--
Michael

--9AxgHkP5UHQ+qFd2
Content-Type: application/pgp-signature; name=signature.asc

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

iQIzBAEBCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmpgHPMACgkQnvQgOdby
QH3LuhAAhCvmF2t5YdrdU6nJ1wxow+EETUqkqKvMqpYbPJOYOjUsp5tYmNBIsv6R
9efScGB0/+QpfmHlHwWSXTFHztSD3Q4ANraQvzzqENzE5/q9JwsYZAcXtFy9nNg8
ukL8eDMQ+Vhow8iPMztfr4aEdbTsVjeB7mmBzRMXogsqUQyApjFqMQusuW9nWHIo
1jRUW8UtL1ZdWU70DId/49OZ8mC2uAdhP+uRbfDgSXwQz3iCOFdG/6xrevUOSgB7
hrGzVHOfRL9WuEjhuvVSO5Y5gKqGLIAhlYswSYxb1qDSlEPk7AEgbW/ZSo6tbnxc
97sMUk1Hz4Q1igmvZxEGpF+JioWpM2D16AxnXKj3CFekCU+cW0G7UbnJPGf4GUjm
HdAMnZ1+D/jQV01DVAYmDrcVpoz26l7uf4qjiUC1w4upyF/8UdYH6gtCAkhLy4Fl
PRA7ESUu8iAG4kNYvbX+sa1Ly7zMTLzR5BGnHkXlTMxDPLBJrexshICtjCwhYkMY
s+d99b7OCiqrINxk9x3nFZ8IpRONSFlm2slBvLId+hEy2zw5sxnbvb0Vbj0LWcxf
tUDqBmd290VU5JyoBwzNekxsJY5tccwNnBxFWkxeFNGGYdWQwGquNQyrJqf1YxkC
jo2rzp7NXAqnKQli0aa4pME2cIEN5UEvJV2n9v9kXtKcqoH1U6M=
=54Gz
-----END PGP SIGNATURE-----

--9AxgHkP5UHQ+qFd2--