From 6b41bd1a459cb457cbb51d021b70004646d78db0 Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Thu, 2 Jul 2026 09:34:17 +0530 Subject: [PATCH] Expand comment on the slot recheck in drop_local_obsolete_slots(). The existing comment explained that a user-created slot could reuse the same shared memory as 'local_slot' during the window between selecting a slot to drop and locking its database, and that we therefore recheck before dropping. It did not, however, spell out the fuller consequence: because local_slot points to a reusable slot-array entry, its fields may already describe a replacement slot, so the earlier drop decision and the slot_database used for locking could relate to an unrelated slot/database. Expand the comment to describe this, and note that the recheck prevents us from dropping a user-created replacement slot while the residual risk (such as briefly locking an unrelated database) is confined to the cycle and is acceptable given the race is rare and non-fatal. No functional change. Author: Fujii Masao Author: Xuneng Zhou Author: Amit Kapila Discussion: https://postgr.es/m/CAHGQGwGGyEDL3dh7uJ6qPsGvnq4QK_R8+U=12CaprnzwrwaLGA@mail.gmail.com Discussion: https://postgr.es/m/CAHGQGwHqQ1PPVFfYKVxLfRyC-byRdwSN0NeaHj9SLYV97oO5cw@mail.gmail.com --- src/backend/replication/logical/slotsync.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c index 05637344363..d1936823506 100644 --- a/src/backend/replication/logical/slotsync.c +++ b/src/backend/replication/logical/slotsync.c @@ -557,9 +557,18 @@ drop_local_obsolete_slots(List *remote_slot_list) * locking the database, there is a possibility of a parallel * database drop by the startup process and the creation of a new * slot by the user. This new user-created slot may end up using - * the same shared memory as that of 'local_slot'. Thus check if - * local_slot is still the synced one before performing the actual - * drop. + * the same shared memory as that of 'local_slot'. + * + * Because local_slot still points to a reusable slot-array entry, + * its fields (name, database OID, invalidation state) may already + * describe such a replacement slot by the time we reach here. + * That means the drop decision made by local_sync_slot_required() + * above could have been based on the replacement slot's data, and + * slot_database could refer to an unrelated database. The recheck + * below keeps us from actually dropping a user-created + * replacement slot; the residual risk is confined to this cycle + * (for example, briefly locking an unrelated database) and is + * acceptable because the race is rare and non-fatal. */ SpinLockAcquire(&local_slot->mutex); synced_slot = local_slot->in_use && local_slot->data.synced;