Hi, On Fri, Aug 7, 2026 at 4:31 AM Ashutosh Sharma <[email protected]> wrote: > > + if (MyReplicationSlot != NULL) > + ReplicationSlotRelease(); > > From this if-condition in AtEOSubXact_ReplicationSlot(), it appears > that even when 'acquiredInSubId == mySubid', 'MyReplicationSlot' could > be NULL and if that ever happens we may/will return from this function > without clearing 'acquiredInSubId'. That matters because > 'currentSubTransactionId' is reset to TopSubTransactionId at each > StartTransaction(), so subxact ids are reused across top-level > transactions; a stale id left behind here could later match an > unrelated subxact. > > AFAIU, in practice this should be unreachable: 'acquiredInSubId' is > only ever set together with 'MyReplicationSlot', and both > ReplicationSlotRelease() and ReplicationSlotDropAcquired() clear it, > so "MyReplicationSlot == NULL" implies "acquiredInSubId == > InvalidSubTransactionId", which can never equal a real `mySubid`. But > the guard's existence suggests you think "MyReplicationSlot == NULL" > is possible. > > So either the reasoning above deserves a comment atop the > if-condition, or, if the NULL case really is impossible, an > 'Assert(MyReplicationSlot != NULL)' would document it more directly > than a silent 'if'.
I get your point. Would something like the below work? + /* + * The aborting subxact is the one that acquired the slot, and its id is + * never invalid, so acquiredInSubId is valid here. It is set only when a + * slot is held, and cleared when the slot is released, so the slot must + * still be held. + */ + Assert(acquiredInSubId != InvalidSubTransactionId); + Assert(MyReplicationSlot != NULL); + ReplicationSlotRelease(); I removed the unnecessary header file inclusions (review comment from Shveta upthread) and attached the v11 patch. -- Bharath Rupireddy Amazon Web Services: https://aws.amazon.com
v11-0001-Fix-replication-slot-leak-on-error-caught-in-a-s.patch
Description: Binary data
