On Sat, Jun 6, 2026 at 3:05 PM Zhijie Hou (Fujitsu) <[email protected]> wrote: > > On Friday, June 5, 2026 8:45 PM Xuneng Zhou <[email protected]> wrote: > > On Wed, Jun 3, 2026 at 8:03 PM Fujii Masao <[email protected]> wrote: > > > > > > On Tue, Jun 2, 2026 at 3:00 PM Xuneng Zhou <[email protected]> > > wrote: > > > > > > /* Drop the local slot if it is not required to be retained. */ > > > if (!local_sync_slot_required(local_slot, remote_slot_list)) > > > { > > > + bool dropped = false; > > > + NameData slot_name = {0}; > > > + Oid slot_database = local_slot->data.database; > > > bool synced_slot; > > > > > > Is it really safe to read slot_database before acquiring the database > > > lock? > > > > Reading slot_database before taking the database lock seems not > > inherently unsafe by itself. The comment suggests that the lock is > > primarily used to prevent conflicts with the startup process running > > ReplicationSlotsDropDBSlots() during db-drop replay; it does not > > protect replication slot array reuse. > > > > The unsafe part could be reading slot_database from local_slot after > > ReplicationSlotControlLock has been released. At this point, the slot > > array cell may already have been freed and reused, so the value read > > may no longer belong to the slot that get_local_synced_slots() > > originally collected. As a result, we could end up locking the wrong > > database. > > > > There seems to be two related issues: > > > > 1) Before drop: reading local_slot->data.database / > > local_slot->data.name after the slot-array lock was released, before > > verifying the cell still represents the same synced slot. > > I recall condition (1) is considered acceptable, since the database lock is > released immediately after re-verifying that the slot is no longer the > original > 'synced' one anyway. Additionally, this race can only occur when replaying a > DROP DATABASE, which is rare in practice. Since we only take a shared lock, it > does not seem to cause real issues. >
It seems that (1) is talking about the access to local_slot->data.name before we acquire database lock in local_sync_slot_required() whereas your response doesn't seem to address that concern. If not, then how exactly does the database lock protect what we are doing in local_sync_slot_required()? -- With Regards, Amit Kapila.
