MIME-Version: 1.0
References: 
 <TY4PR01MB177184FF9EE916F577E1F554194082@TY4PR01MB17718.jpnprd01.prod.outlook.com>
In-Reply-To: 
 <TY4PR01MB177184FF9EE916F577E1F554194082@TY4PR01MB17718.jpnprd01.prod.outlook.com>
From: Fujii Masao <masao.fujii@gmail.com>
Date: Fri, 29 May 2026 17:09:44 +0900
Message-ID: 
 <CAHGQGwHqQ1PPVFfYKVxLfRyC-byRdwSN0NeaHj9SLYV97oO5cw@mail.gmail.com>
Subject: Re: Fix race in ReplicationSlotRelease for ephemeral slots
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAHGQGwHqQ1PPVFfYKVxLfRyC-byRdwSN0NeaHj9SLYV97oO5cw%40mail.gmail.com>
Precedence: bulk

On Wed, May 27, 2026 at 8:50=E2=80=AFPM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> Hi,
>
> While testing the slot release logic, I noticed a bug in
> ReplicationSlotRelease() where it may access a replication slot array ent=
ry that
> has already been released by itself.
>
> The detail is: When releasing an ephemeral replication slot,
> ReplicationSlotRelease() first drops the slot via ReplicationSlotDropAcqu=
ired().
> After this point, the slot's shared memory slot array entry can be immedi=
ately
> reused by another backend creating a new slot.
>
> However, ReplicationSlotRelease() continued executing common cleanup code=
 that
> still dereferenced the old slot pointer and updated shared memory fields =
such as
> effective_xmin. If the slot array entry had already been reallocated, the=
se
> writes could inadvertently affect a different, unrelated slot.

Good catch!


> I am attaching a patch that avoids touching slot shared-memory state afte=
r
> dropping an ephemeral slot. Keep the post-release shared-memory updates o=
nly for
> non-ephemeral slots, where the slot remains valid after release.

Thanks for the patch! It looks good to me.
Barring any objections, I will commit it.


> I haven't attached a test for this fix, as the change is straightforward =
and the
> likelihood of encountering this bug is low, so it may not be worth adding=
 test
> cycles for it.

+1

Regards,

--=20
Fujii Masao