Re: injection_points: canceled or terminated waiters leak their wait slots

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.
>
> 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