On 01/03/2024 12:15, Bertrand Drouvot wrote:
Hi hackers,
I think that pgstat_reset_replslot() is missing LWLock protection. Indeed, we
don't have any guarantee that the slot is active (then preventing it to be
dropped/recreated) when this function is executed.
Yes, so it seems at quick glance. We have a similar issue in
pgstat_fetch_replslot(); it might return stats for wrong slot, if the
slot is dropped/recreated concurrently. Do we care?
--- a/src/backend/utils/activity/pgstat_replslot.c
+++ b/src/backend/utils/activity/pgstat_replslot.c
@@ -46,6 +46,8 @@ pgstat_reset_replslot(const char *name)
Assert(name != NULL);
+ LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
+
/* Check if the slot exits with the given name. */
slot = SearchNamedReplicationSlot(name, true);
SearchNamedReplicationSlot() will also acquire the lock in LW_SHARED
mode, when you pass need_lock=true. So that at least should be changed
to false.
--
Heikki Linnakangas
Neon (https://neon.tech)