public inbox for [email protected]  
help / color / mirror / Atom feed
From: Fujii Masao <[email protected]>
To: Zhijie Hou (Fujitsu) <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Fix race in ReplicationSlotRelease for ephemeral slots
Date: Fri, 29 May 2026 22:31:31 +0900
Message-ID: <CAHGQGwEkkYchHB=EaArhRh25y5exhFxKxV7ZeVyG-uxUair=kw@mail.gmail.com> (raw)
In-Reply-To: <CAHGQGwHqQ1PPVFfYKVxLfRyC-byRdwSN0NeaHj9SLYV97oO5cw@mail.gmail.com>
References: <TY4PR01MB177184FF9EE916F577E1F554194082@TY4PR01MB17718.jpnprd01.prod.outlook.com>
	<CAHGQGwHqQ1PPVFfYKVxLfRyC-byRdwSN0NeaHj9SLYV97oO5cw@mail.gmail.com>

On Fri, May 29, 2026 at 5:09 PM Fujii Masao <[email protected]> wrote:
> > I am attaching a patch that avoids touching slot shared-memory state after
> > dropping an ephemeral slot. Keep the post-release shared-memory updates only 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.

Since this fix should be backpatched to all supported branches,
I prepared patches for them. Attached.

Regards,

-- 
Fujii Masao

From 56e8c1068e19c3ccd41cecb75b1ee26affb52abf Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 29 May 2026 18:43:50 +0900
Subject: [PATCH v1] Fix race in ReplicationSlotRelease for ephemeral slots

When releasing an ephemeral replication slot, ReplicationSlotRelease() first
drops the slot via ReplicationSlotDropAcquired(). After this point, the slot's
shared memory slot array entry can be immediately 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, these
writes could inadvertently affect a different, unrelated slot.

This commit avoids touching slot shared-memory state after dropping an ephemeral
slot. Keep the post-release shared-memory updates only for non-ephemeral slots,
where the slot remains valid after release.
---
 src/backend/replication/slot.c | 68 +++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 33 deletions(-)

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 4246d0a51e1..e60ccc424dd 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -737,44 +737,46 @@ ReplicationSlotRelease(void)
 		 */
 		ReplicationSlotDropAcquired();
 	}
-
-	/*
-	 * If slot needed to temporarily restrain both data and catalog xmin to
-	 * create the catalog snapshot, remove that temporary constraint.
-	 * Snapshots can only be exported while the initial snapshot is still
-	 * acquired.
-	 */
-	if (!TransactionIdIsValid(slot->data.xmin) &&
-		TransactionIdIsValid(slot->effective_xmin))
+	else
 	{
-		SpinLockAcquire(&slot->mutex);
-		slot->effective_xmin = InvalidTransactionId;
-		SpinLockRelease(&slot->mutex);
-		ReplicationSlotsComputeRequiredXmin(false);
-	}
-
-	/*
-	 * Set the time since the slot has become inactive. We get the current
-	 * time beforehand to avoid system call while holding the spinlock.
-	 */
-	now = GetCurrentTimestamp();
+		/*
+		 * If slot needed to temporarily restrain both data and catalog xmin
+		 * to create the catalog snapshot, remove that temporary constraint.
+		 * Snapshots can only be exported while the initial snapshot is still
+		 * acquired.
+		 */
+		if (!TransactionIdIsValid(slot->data.xmin) &&
+			TransactionIdIsValid(slot->effective_xmin))
+		{
+			SpinLockAcquire(&slot->mutex);
+			slot->effective_xmin = InvalidTransactionId;
+			SpinLockRelease(&slot->mutex);
+			ReplicationSlotsComputeRequiredXmin(false);
+		}
 
-	if (slot->data.persistency == RS_PERSISTENT)
-	{
 		/*
-		 * Mark persistent slot inactive.  We're not freeing it, just
-		 * disconnecting, but wake up others that may be waiting for it.
+		 * Set the time since the slot has become inactive. We get the current
+		 * time beforehand to avoid system call while holding the spinlock.
 		 */
-		SpinLockAcquire(&slot->mutex);
-		slot->active_pid = 0;
-		ReplicationSlotSetInactiveSince(slot, now, false);
-		SpinLockRelease(&slot->mutex);
-		ConditionVariableBroadcast(&slot->active_cv);
-	}
-	else
-		ReplicationSlotSetInactiveSince(slot, now, true);
+		now = GetCurrentTimestamp();
 
-	MyReplicationSlot = NULL;
+		if (slot->data.persistency == RS_PERSISTENT)
+		{
+			/*
+			 * Mark persistent slot inactive.  We're not freeing it, just
+			 * disconnecting, but wake up others that may be waiting for it.
+			 */
+			SpinLockAcquire(&slot->mutex);
+			slot->active_pid = 0;
+			ReplicationSlotSetInactiveSince(slot, now, false);
+			SpinLockRelease(&slot->mutex);
+			ConditionVariableBroadcast(&slot->active_cv);
+		}
+		else
+			ReplicationSlotSetInactiveSince(slot, now, true);
+
+		MyReplicationSlot = NULL;
+	}
 
 	/* might not have been set when we've been a plain slot */
 	LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
-- 
2.53.0


From 8ee2b85c2a68e6efc68e517e0d20e6cbc96eadbd Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 29 May 2026 18:43:50 +0900
Subject: [PATCH v1] Fix race in ReplicationSlotRelease for ephemeral slots

When releasing an ephemeral replication slot, ReplicationSlotRelease() first
drops the slot via ReplicationSlotDropAcquired(). After this point, the slot's
shared memory slot array entry can be immediately 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, these
writes could inadvertently affect a different, unrelated slot.

This commit avoids touching slot shared-memory state after dropping an ephemeral
slot. Keep the post-release shared-memory updates only for non-ephemeral slots,
where the slot remains valid after release.
---
 src/backend/replication/slot.c | 52 ++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index aa017451ccc..2034722f0a4 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -589,35 +589,37 @@ ReplicationSlotRelease(void)
 		 */
 		ReplicationSlotDropAcquired();
 	}
-
-	/*
-	 * If slot needed to temporarily restrain both data and catalog xmin to
-	 * create the catalog snapshot, remove that temporary constraint.
-	 * Snapshots can only be exported while the initial snapshot is still
-	 * acquired.
-	 */
-	if (!TransactionIdIsValid(slot->data.xmin) &&
-		TransactionIdIsValid(slot->effective_xmin))
-	{
-		SpinLockAcquire(&slot->mutex);
-		slot->effective_xmin = InvalidTransactionId;
-		SpinLockRelease(&slot->mutex);
-		ReplicationSlotsComputeRequiredXmin(false);
-	}
-
-	if (slot->data.persistency == RS_PERSISTENT)
+	else
 	{
 		/*
-		 * Mark persistent slot inactive.  We're not freeing it, just
-		 * disconnecting, but wake up others that may be waiting for it.
+		 * If slot needed to temporarily restrain both data and catalog xmin
+		 * to create the catalog snapshot, remove that temporary constraint.
+		 * Snapshots can only be exported while the initial snapshot is still
+		 * acquired.
 		 */
-		SpinLockAcquire(&slot->mutex);
-		slot->active_pid = 0;
-		SpinLockRelease(&slot->mutex);
-		ConditionVariableBroadcast(&slot->active_cv);
-	}
+		if (!TransactionIdIsValid(slot->data.xmin) &&
+			TransactionIdIsValid(slot->effective_xmin))
+		{
+			SpinLockAcquire(&slot->mutex);
+			slot->effective_xmin = InvalidTransactionId;
+			SpinLockRelease(&slot->mutex);
+			ReplicationSlotsComputeRequiredXmin(false);
+		}
 
-	MyReplicationSlot = NULL;
+		if (slot->data.persistency == RS_PERSISTENT)
+		{
+			/*
+			 * Mark persistent slot inactive.  We're not freeing it, just
+			 * disconnecting, but wake up others that may be waiting for it.
+			 */
+			SpinLockAcquire(&slot->mutex);
+			slot->active_pid = 0;
+			SpinLockRelease(&slot->mutex);
+			ConditionVariableBroadcast(&slot->active_cv);
+		}
+
+		MyReplicationSlot = NULL;
+	}
 
 	/* might not have been set when we've been a plain slot */
 	LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
-- 
2.53.0


From 28da67621c97824730030094cdfd1ad02b2655e8 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 29 May 2026 18:43:50 +0900
Subject: [PATCH v1] Fix race in ReplicationSlotRelease for ephemeral slots

When releasing an ephemeral replication slot, ReplicationSlotRelease() first
drops the slot via ReplicationSlotDropAcquired(). After this point, the slot's
shared memory slot array entry can be immediately 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, these
writes could inadvertently affect a different, unrelated slot.

This commit avoids touching slot shared-memory state after dropping an ephemeral
slot. Keep the post-release shared-memory updates only for non-ephemeral slots,
where the slot remains valid after release.
---
 src/backend/replication/slot.c | 76 +++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 37 deletions(-)

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 4e54a8d0f14..2f1696f4111 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -702,48 +702,50 @@ ReplicationSlotRelease(void)
 		 */
 		ReplicationSlotDropAcquired();
 	}
-
-	/*
-	 * If slot needed to temporarily restrain both data and catalog xmin to
-	 * create the catalog snapshot, remove that temporary constraint.
-	 * Snapshots can only be exported while the initial snapshot is still
-	 * acquired.
-	 */
-	if (!TransactionIdIsValid(slot->data.xmin) &&
-		TransactionIdIsValid(slot->effective_xmin))
+	else
 	{
-		SpinLockAcquire(&slot->mutex);
-		slot->effective_xmin = InvalidTransactionId;
-		SpinLockRelease(&slot->mutex);
-		ReplicationSlotsComputeRequiredXmin(false);
-	}
-
-	/*
-	 * Set the time since the slot has become inactive. We get the current
-	 * time beforehand to avoid system call while holding the spinlock.
-	 */
-	now = GetCurrentTimestamp();
+		/*
+		 * If slot needed to temporarily restrain both data and catalog xmin
+		 * to create the catalog snapshot, remove that temporary constraint.
+		 * Snapshots can only be exported while the initial snapshot is still
+		 * acquired.
+		 */
+		if (!TransactionIdIsValid(slot->data.xmin) &&
+			TransactionIdIsValid(slot->effective_xmin))
+		{
+			SpinLockAcquire(&slot->mutex);
+			slot->effective_xmin = InvalidTransactionId;
+			SpinLockRelease(&slot->mutex);
+			ReplicationSlotsComputeRequiredXmin(false);
+		}
 
-	if (slot->data.persistency == RS_PERSISTENT)
-	{
 		/*
-		 * Mark persistent slot inactive.  We're not freeing it, just
-		 * disconnecting, but wake up others that may be waiting for it.
+		 * Set the time since the slot has become inactive. We get the current
+		 * time beforehand to avoid system call while holding the spinlock.
 		 */
-		SpinLockAcquire(&slot->mutex);
-		slot->active_pid = 0;
-		slot->inactive_since = now;
-		SpinLockRelease(&slot->mutex);
-		ConditionVariableBroadcast(&slot->active_cv);
-	}
-	else
-	{
-		SpinLockAcquire(&slot->mutex);
-		slot->inactive_since = now;
-		SpinLockRelease(&slot->mutex);
-	}
+		now = GetCurrentTimestamp();
 
-	MyReplicationSlot = NULL;
+		if (slot->data.persistency == RS_PERSISTENT)
+		{
+			/*
+			 * Mark persistent slot inactive.  We're not freeing it, just
+			 * disconnecting, but wake up others that may be waiting for it.
+			 */
+			SpinLockAcquire(&slot->mutex);
+			slot->active_pid = 0;
+			slot->inactive_since = now;
+			SpinLockRelease(&slot->mutex);
+			ConditionVariableBroadcast(&slot->active_cv);
+		}
+		else
+		{
+			SpinLockAcquire(&slot->mutex);
+			slot->inactive_since = now;
+			SpinLockRelease(&slot->mutex);
+		}
+
+		MyReplicationSlot = NULL;
+	}
 
 	/* might not have been set when we've been a plain slot */
 	LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
-- 
2.53.0



Attachments:

  [text/plain] pg18-v1-0001-Fix-race-in-ReplicationSlotRelease-for-ephemeral-.txt (3.8K, 2-pg18-v1-0001-Fix-race-in-ReplicationSlotRelease-for-ephemeral-.txt)
  download | inline diff:
From 56e8c1068e19c3ccd41cecb75b1ee26affb52abf Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 29 May 2026 18:43:50 +0900
Subject: [PATCH v1] Fix race in ReplicationSlotRelease for ephemeral slots

When releasing an ephemeral replication slot, ReplicationSlotRelease() first
drops the slot via ReplicationSlotDropAcquired(). After this point, the slot's
shared memory slot array entry can be immediately 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, these
writes could inadvertently affect a different, unrelated slot.

This commit avoids touching slot shared-memory state after dropping an ephemeral
slot. Keep the post-release shared-memory updates only for non-ephemeral slots,
where the slot remains valid after release.
---
 src/backend/replication/slot.c | 68 +++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 33 deletions(-)

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 4246d0a51e1..e60ccc424dd 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -737,44 +737,46 @@ ReplicationSlotRelease(void)
 		 */
 		ReplicationSlotDropAcquired();
 	}
-
-	/*
-	 * If slot needed to temporarily restrain both data and catalog xmin to
-	 * create the catalog snapshot, remove that temporary constraint.
-	 * Snapshots can only be exported while the initial snapshot is still
-	 * acquired.
-	 */
-	if (!TransactionIdIsValid(slot->data.xmin) &&
-		TransactionIdIsValid(slot->effective_xmin))
+	else
 	{
-		SpinLockAcquire(&slot->mutex);
-		slot->effective_xmin = InvalidTransactionId;
-		SpinLockRelease(&slot->mutex);
-		ReplicationSlotsComputeRequiredXmin(false);
-	}
-
-	/*
-	 * Set the time since the slot has become inactive. We get the current
-	 * time beforehand to avoid system call while holding the spinlock.
-	 */
-	now = GetCurrentTimestamp();
+		/*
+		 * If slot needed to temporarily restrain both data and catalog xmin
+		 * to create the catalog snapshot, remove that temporary constraint.
+		 * Snapshots can only be exported while the initial snapshot is still
+		 * acquired.
+		 */
+		if (!TransactionIdIsValid(slot->data.xmin) &&
+			TransactionIdIsValid(slot->effective_xmin))
+		{
+			SpinLockAcquire(&slot->mutex);
+			slot->effective_xmin = InvalidTransactionId;
+			SpinLockRelease(&slot->mutex);
+			ReplicationSlotsComputeRequiredXmin(false);
+		}
 
-	if (slot->data.persistency == RS_PERSISTENT)
-	{
 		/*
-		 * Mark persistent slot inactive.  We're not freeing it, just
-		 * disconnecting, but wake up others that may be waiting for it.
+		 * Set the time since the slot has become inactive. We get the current
+		 * time beforehand to avoid system call while holding the spinlock.
 		 */
-		SpinLockAcquire(&slot->mutex);
-		slot->active_pid = 0;
-		ReplicationSlotSetInactiveSince(slot, now, false);
-		SpinLockRelease(&slot->mutex);
-		ConditionVariableBroadcast(&slot->active_cv);
-	}
-	else
-		ReplicationSlotSetInactiveSince(slot, now, true);
+		now = GetCurrentTimestamp();
 
-	MyReplicationSlot = NULL;
+		if (slot->data.persistency == RS_PERSISTENT)
+		{
+			/*
+			 * Mark persistent slot inactive.  We're not freeing it, just
+			 * disconnecting, but wake up others that may be waiting for it.
+			 */
+			SpinLockAcquire(&slot->mutex);
+			slot->active_pid = 0;
+			ReplicationSlotSetInactiveSince(slot, now, false);
+			SpinLockRelease(&slot->mutex);
+			ConditionVariableBroadcast(&slot->active_cv);
+		}
+		else
+			ReplicationSlotSetInactiveSince(slot, now, true);
+
+		MyReplicationSlot = NULL;
+	}
 
 	/* might not have been set when we've been a plain slot */
 	LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
-- 
2.53.0



  [text/plain] pg14-pg16-v1-0001-Fix-race-in-ReplicationSlotRelease-for-ephemeral-.txt (3.2K, 3-pg14-pg16-v1-0001-Fix-race-in-ReplicationSlotRelease-for-ephemeral-.txt)
  download | inline diff:
From 8ee2b85c2a68e6efc68e517e0d20e6cbc96eadbd Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 29 May 2026 18:43:50 +0900
Subject: [PATCH v1] Fix race in ReplicationSlotRelease for ephemeral slots

When releasing an ephemeral replication slot, ReplicationSlotRelease() first
drops the slot via ReplicationSlotDropAcquired(). After this point, the slot's
shared memory slot array entry can be immediately 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, these
writes could inadvertently affect a different, unrelated slot.

This commit avoids touching slot shared-memory state after dropping an ephemeral
slot. Keep the post-release shared-memory updates only for non-ephemeral slots,
where the slot remains valid after release.
---
 src/backend/replication/slot.c | 52 ++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index aa017451ccc..2034722f0a4 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -589,35 +589,37 @@ ReplicationSlotRelease(void)
 		 */
 		ReplicationSlotDropAcquired();
 	}
-
-	/*
-	 * If slot needed to temporarily restrain both data and catalog xmin to
-	 * create the catalog snapshot, remove that temporary constraint.
-	 * Snapshots can only be exported while the initial snapshot is still
-	 * acquired.
-	 */
-	if (!TransactionIdIsValid(slot->data.xmin) &&
-		TransactionIdIsValid(slot->effective_xmin))
-	{
-		SpinLockAcquire(&slot->mutex);
-		slot->effective_xmin = InvalidTransactionId;
-		SpinLockRelease(&slot->mutex);
-		ReplicationSlotsComputeRequiredXmin(false);
-	}
-
-	if (slot->data.persistency == RS_PERSISTENT)
+	else
 	{
 		/*
-		 * Mark persistent slot inactive.  We're not freeing it, just
-		 * disconnecting, but wake up others that may be waiting for it.
+		 * If slot needed to temporarily restrain both data and catalog xmin
+		 * to create the catalog snapshot, remove that temporary constraint.
+		 * Snapshots can only be exported while the initial snapshot is still
+		 * acquired.
 		 */
-		SpinLockAcquire(&slot->mutex);
-		slot->active_pid = 0;
-		SpinLockRelease(&slot->mutex);
-		ConditionVariableBroadcast(&slot->active_cv);
-	}
+		if (!TransactionIdIsValid(slot->data.xmin) &&
+			TransactionIdIsValid(slot->effective_xmin))
+		{
+			SpinLockAcquire(&slot->mutex);
+			slot->effective_xmin = InvalidTransactionId;
+			SpinLockRelease(&slot->mutex);
+			ReplicationSlotsComputeRequiredXmin(false);
+		}
 
-	MyReplicationSlot = NULL;
+		if (slot->data.persistency == RS_PERSISTENT)
+		{
+			/*
+			 * Mark persistent slot inactive.  We're not freeing it, just
+			 * disconnecting, but wake up others that may be waiting for it.
+			 */
+			SpinLockAcquire(&slot->mutex);
+			slot->active_pid = 0;
+			SpinLockRelease(&slot->mutex);
+			ConditionVariableBroadcast(&slot->active_cv);
+		}
+
+		MyReplicationSlot = NULL;
+	}
 
 	/* might not have been set when we've been a plain slot */
 	LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
-- 
2.53.0



  [text/plain] pg17-v1-0001-Fix-race-in-ReplicationSlotRelease-for-ephemeral-.txt (3.9K, 4-pg17-v1-0001-Fix-race-in-ReplicationSlotRelease-for-ephemeral-.txt)
  download | inline diff:
From 28da67621c97824730030094cdfd1ad02b2655e8 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 29 May 2026 18:43:50 +0900
Subject: [PATCH v1] Fix race in ReplicationSlotRelease for ephemeral slots

When releasing an ephemeral replication slot, ReplicationSlotRelease() first
drops the slot via ReplicationSlotDropAcquired(). After this point, the slot's
shared memory slot array entry can be immediately 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, these
writes could inadvertently affect a different, unrelated slot.

This commit avoids touching slot shared-memory state after dropping an ephemeral
slot. Keep the post-release shared-memory updates only for non-ephemeral slots,
where the slot remains valid after release.
---
 src/backend/replication/slot.c | 76 +++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 37 deletions(-)

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 4e54a8d0f14..2f1696f4111 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -702,48 +702,50 @@ ReplicationSlotRelease(void)
 		 */
 		ReplicationSlotDropAcquired();
 	}
-
-	/*
-	 * If slot needed to temporarily restrain both data and catalog xmin to
-	 * create the catalog snapshot, remove that temporary constraint.
-	 * Snapshots can only be exported while the initial snapshot is still
-	 * acquired.
-	 */
-	if (!TransactionIdIsValid(slot->data.xmin) &&
-		TransactionIdIsValid(slot->effective_xmin))
+	else
 	{
-		SpinLockAcquire(&slot->mutex);
-		slot->effective_xmin = InvalidTransactionId;
-		SpinLockRelease(&slot->mutex);
-		ReplicationSlotsComputeRequiredXmin(false);
-	}
-
-	/*
-	 * Set the time since the slot has become inactive. We get the current
-	 * time beforehand to avoid system call while holding the spinlock.
-	 */
-	now = GetCurrentTimestamp();
+		/*
+		 * If slot needed to temporarily restrain both data and catalog xmin
+		 * to create the catalog snapshot, remove that temporary constraint.
+		 * Snapshots can only be exported while the initial snapshot is still
+		 * acquired.
+		 */
+		if (!TransactionIdIsValid(slot->data.xmin) &&
+			TransactionIdIsValid(slot->effective_xmin))
+		{
+			SpinLockAcquire(&slot->mutex);
+			slot->effective_xmin = InvalidTransactionId;
+			SpinLockRelease(&slot->mutex);
+			ReplicationSlotsComputeRequiredXmin(false);
+		}
 
-	if (slot->data.persistency == RS_PERSISTENT)
-	{
 		/*
-		 * Mark persistent slot inactive.  We're not freeing it, just
-		 * disconnecting, but wake up others that may be waiting for it.
+		 * Set the time since the slot has become inactive. We get the current
+		 * time beforehand to avoid system call while holding the spinlock.
 		 */
-		SpinLockAcquire(&slot->mutex);
-		slot->active_pid = 0;
-		slot->inactive_since = now;
-		SpinLockRelease(&slot->mutex);
-		ConditionVariableBroadcast(&slot->active_cv);
-	}
-	else
-	{
-		SpinLockAcquire(&slot->mutex);
-		slot->inactive_since = now;
-		SpinLockRelease(&slot->mutex);
-	}
+		now = GetCurrentTimestamp();
 
-	MyReplicationSlot = NULL;
+		if (slot->data.persistency == RS_PERSISTENT)
+		{
+			/*
+			 * Mark persistent slot inactive.  We're not freeing it, just
+			 * disconnecting, but wake up others that may be waiting for it.
+			 */
+			SpinLockAcquire(&slot->mutex);
+			slot->active_pid = 0;
+			slot->inactive_since = now;
+			SpinLockRelease(&slot->mutex);
+			ConditionVariableBroadcast(&slot->active_cv);
+		}
+		else
+		{
+			SpinLockAcquire(&slot->mutex);
+			slot->inactive_since = now;
+			SpinLockRelease(&slot->mutex);
+		}
+
+		MyReplicationSlot = NULL;
+	}
 
 	/* might not have been set when we've been a plain slot */
 	LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
-- 
2.53.0



view thread (27+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Fix race in ReplicationSlotRelease for ephemeral slots
  In-Reply-To: <CAHGQGwEkkYchHB=EaArhRh25y5exhFxKxV7ZeVyG-uxUair=kw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox