Date: Mon, 24 Nov 2025 12:36:25 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Nathan Bossart <nathan@postgresql.org>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pgsql: Teach DSM registry to ERROR if attaching to an
 uninitialized ent
Message-ID: <aSSlqWwLVuXMmN-o@nathan>
References: <E1vJHUk-006I7r-37@gemulon.postgresql.org>
 <CA+TgmoZGFzLJpYANBG9yDyoakVm-P2Acdker-Y+DHvPWfvBE5w@mail.gmail.com>
 <aR89_9oYFHNtUTFa@nathan>
 <CA+TgmoZCHER55cGmnL_svMjH=k-S0w29ih+ym_+=t032TSsOFA@mail.gmail.com>
 <aR-fqzfZYSd3TVKa@nathan>
 <CA+TgmoZsxbVhoxNGTWSd6=vHvyoqOSDNXJZabBZx+St8uPduLQ@mail.gmail.com>
 <aSIR4Z4U887f_KcQ@nathan>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="RrQwG/d5bOC9SOJ1"
Content-Disposition: inline
In-Reply-To: <aSIR4Z4U887f_KcQ@nathan>
Archived-At: <https://www.postgresql.org/message-id/aSSlqWwLVuXMmN-o%40nathan>
Precedence: bulk


--RrQwG/d5bOC9SOJ1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sat, Nov 22, 2025 at 01:41:21PM -0600, Nathan Bossart wrote:
> I'll give what you suggested a try.  It seems reasonable enough.

Here is a mostly-untested first try.

-- 
nathan

--RrQwG/d5bOC9SOJ1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v1-0001-Revert-Teach-DSM-registry-to-ERROR-if-attaching-t.patch

From e639d70a10f6288034ebe433a22b9dbc53562b01 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 24 Nov 2025 11:03:52 -0600
Subject: [PATCH v1 1/2] Revert "Teach DSM registry to ERROR if attaching to an
 uninitialized entry."

This reverts commit 1165a933aab1355757a43cfd9193b6cce06f573b.
---
 src/backend/storage/ipc/dsm_registry.c | 34 +++-----------------------
 1 file changed, 4 insertions(+), 30 deletions(-)

diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c
index 7eba8a8cffb..a926b9c3f32 100644
--- a/src/backend/storage/ipc/dsm_registry.c
+++ b/src/backend/storage/ipc/dsm_registry.c
@@ -93,7 +93,6 @@ typedef struct DSMRegistryEntry
 {
 	char		name[NAMEDATALEN];
 	DSMREntryType type;
-	bool		initialized;
 	union
 	{
 		NamedDSMState dsm;
@@ -217,7 +216,6 @@ GetNamedDSMSegment(const char *name, size_t size,
 		dsm_segment *seg;
 
 		entry->type = DSMR_ENTRY_TYPE_DSM;
-		entry->initialized = false;
 
 		/* Initialize the segment. */
 		seg = dsm_create(size, 0);
@@ -230,21 +228,13 @@ GetNamedDSMSegment(const char *name, size_t size,
 
 		if (init_callback)
 			(*init_callback) (ret);
-
-		entry->initialized = true;
 	}
 	else if (entry->type != DSMR_ENTRY_TYPE_DSM)
 		ereport(ERROR,
-				(errmsg("requested DSM segment \"%s\" does not match type of existing entry",
-						name)));
-	else if (!entry->initialized)
-		ereport(ERROR,
-				(errmsg("requested DSM segment \"%s\" failed initialization",
-						name)));
+				(errmsg("requested DSM segment does not match type of existing entry")));
 	else if (entry->dsm.size != size)
 		ereport(ERROR,
-				(errmsg("requested DSM segment \"%s\" does not match size of existing entry",
-						name)));
+				(errmsg("requested DSM segment size does not match size of existing segment")));
 	else
 	{
 		NamedDSMState *state = &entry->dsm;
@@ -307,7 +297,6 @@ GetNamedDSA(const char *name, bool *found)
 		NamedDSAState *state = &entry->dsa;
 
 		entry->type = DSMR_ENTRY_TYPE_DSA;
-		entry->initialized = false;
 
 		/* Initialize the LWLock tranche for the DSA. */
 		state->tranche = LWLockNewTrancheId(name);
@@ -319,17 +308,10 @@ GetNamedDSA(const char *name, bool *found)
 
 		/* Store handle for other backends to use. */
 		state->handle = dsa_get_handle(ret);
-
-		entry->initialized = true;
 	}
 	else if (entry->type != DSMR_ENTRY_TYPE_DSA)
 		ereport(ERROR,
-				(errmsg("requested DSA \"%s\" does not match type of existing entry",
-						name)));
-	else if (!entry->initialized)
-		ereport(ERROR,
-				(errmsg("requested DSA \"%s\" failed initialization",
-						name)));
+				(errmsg("requested DSA does not match type of existing entry")));
 	else
 	{
 		NamedDSAState *state = &entry->dsa;
@@ -390,7 +372,6 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found)
 		dsa_area   *dsa;
 
 		entry->type = DSMR_ENTRY_TYPE_DSH;
-		entry->initialized = false;
 
 		/* Initialize the LWLock tranche for the hash table. */
 		dsh_state->tranche = LWLockNewTrancheId(name);
@@ -408,17 +389,10 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found)
 		/* Store handles for other backends to use. */
 		dsh_state->dsa_handle = dsa_get_handle(dsa);
 		dsh_state->dsh_handle = dshash_get_hash_table_handle(ret);
-
-		entry->initialized = true;
 	}
 	else if (entry->type != DSMR_ENTRY_TYPE_DSH)
 		ereport(ERROR,
-				(errmsg("requested DSHash \"%s\" does not match type of existing entry",
-						name)));
-	else if (!entry->initialized)
-		ereport(ERROR,
-				(errmsg("requested DSHash \"%s\" failed initialization",
-						name)));
+				(errmsg("requested DSHash does not match type of existing entry")));
 	else
 	{
 		NamedDSHState *dsh_state = &entry->dsh;
-- 
2.39.5 (Apple Git-154)


--RrQwG/d5bOC9SOJ1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v1-0002-handle-ERRORs-in-DSM-registry-functions.patch

From 6da47303c5e5acaca4cc9808316a0457f6c6f0af Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 24 Nov 2025 12:28:23 -0600
Subject: [PATCH v1 2/2] handle ERRORs in DSM registry functions

---
 src/backend/storage/ipc/dsm_registry.c | 129 ++++++++++++++++++++++---
 1 file changed, 115 insertions(+), 14 deletions(-)

diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c
index a926b9c3f32..7d1b584d166 100644
--- a/src/backend/storage/ipc/dsm_registry.c
+++ b/src/backend/storage/ipc/dsm_registry.c
@@ -154,10 +154,24 @@ init_dsm_registry(void)
 	if (DSMRegistryCtx->dshh == DSHASH_HANDLE_INVALID)
 	{
 		/* Initialize dynamic shared hash table for registry. */
-		dsm_registry_dsa = dsa_create(LWTRANCHE_DSM_REGISTRY_DSA);
+		PG_TRY();
+		{
+			dsm_registry_dsa = dsa_create(LWTRANCHE_DSM_REGISTRY_DSA);
+			dsm_registry_table = dshash_create(dsm_registry_dsa, &dsh_params, NULL);
+		}
+		PG_CATCH();
+		{
+			if (dsm_registry_dsa)
+			{
+				dsa_detach(dsm_registry_dsa);
+				dsm_registry_dsa = NULL;
+			}
+			PG_RE_THROW();
+		}
+		PG_END_TRY();
+
 		dsa_pin(dsm_registry_dsa);
 		dsa_pin_mapping(dsm_registry_dsa);
-		dsm_registry_table = dshash_create(dsm_registry_dsa, &dsh_params, NULL);
 
 		/* Store handles in shared memory for other backends to use. */
 		DSMRegistryCtx->dsah = dsa_get_handle(dsm_registry_dsa);
@@ -216,18 +230,38 @@ GetNamedDSMSegment(const char *name, size_t size,
 		dsm_segment *seg;
 
 		entry->type = DSMR_ENTRY_TYPE_DSM;
+		state->handle = DSM_HANDLE_INVALID;
 
 		/* Initialize the segment. */
-		seg = dsm_create(size, 0);
+		PG_TRY();
+		{
+			seg = dsm_create(size, 0);
+		}
+		PG_CATCH();
+		{
+			dshash_delete_entry(dsm_registry_table, entry);
+			PG_RE_THROW();
+		}
+		PG_END_TRY();
+
+		PG_TRY();
+		{
+			ret = dsm_segment_address(seg);
+			if (init_callback)
+				(*init_callback) (ret);
+		}
+		PG_CATCH();
+		{
+			dsm_detach(seg);
+			dshash_delete_entry(dsm_registry_table, entry);
+			PG_RE_THROW();
+		}
+		PG_END_TRY();
 
 		dsm_pin_segment(seg);
 		dsm_pin_mapping(seg);
 		state->handle = dsm_segment_handle(seg);
 		state->size = size;
-		ret = dsm_segment_address(seg);
-
-		if (init_callback)
-			(*init_callback) (ret);
 	}
 	else if (entry->type != DSMR_ENTRY_TYPE_DSM)
 		ereport(ERROR,
@@ -297,9 +331,19 @@ GetNamedDSA(const char *name, bool *found)
 		NamedDSAState *state = &entry->dsa;
 
 		entry->type = DSMR_ENTRY_TYPE_DSA;
+		state->handle = DSA_HANDLE_INVALID;
 
 		/* Initialize the LWLock tranche for the DSA. */
-		state->tranche = LWLockNewTrancheId(name);
+		PG_TRY();
+		{
+			state->tranche = LWLockNewTrancheId(name);
+		}
+		PG_CATCH();
+		{
+			dshash_delete_entry(dsm_registry_table, entry);
+			PG_RE_THROW();
+		}
+		PG_END_TRY();
 
 		/* Initialize the DSA. */
 		ret = dsa_create(state->tranche);
@@ -312,6 +356,16 @@ GetNamedDSA(const char *name, bool *found)
 	else if (entry->type != DSMR_ENTRY_TYPE_DSA)
 		ereport(ERROR,
 				(errmsg("requested DSA does not match type of existing entry")));
+	else if (entry->dsa.handle == DSA_HANDLE_INVALID)
+	{
+		NamedDSAState *state = &entry->dsa;
+
+		ret = dsa_create(state->tranche);
+
+		dsa_pin(ret);
+		dsa_pin_mapping(ret);
+		state->handle = dsa_get_handle(ret);
+	}
 	else
 	{
 		NamedDSAState *state = &entry->dsa;
@@ -372,19 +426,40 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found)
 		dsa_area   *dsa;
 
 		entry->type = DSMR_ENTRY_TYPE_DSH;
+		dsh_state->dsa_handle = DSA_HANDLE_INVALID;
+		dsh_state->dsh_handle = DSHASH_HANDLE_INVALID;
 
 		/* Initialize the LWLock tranche for the hash table. */
-		dsh_state->tranche = LWLockNewTrancheId(name);
+		PG_TRY();
+		{
+			dsh_state->tranche = LWLockNewTrancheId(name);
+		}
+		PG_CATCH();
+		{
+			dshash_delete_entry(dsm_registry_table, entry);
+			PG_RE_THROW();
+		}
+		PG_END_TRY();
 
 		/* Initialize the DSA for the hash table. */
 		dsa = dsa_create(dsh_state->tranche);
-		dsa_pin(dsa);
-		dsa_pin_mapping(dsa);
 
 		/* Initialize the dshash table. */
-		memcpy(&params_copy, params, sizeof(dshash_parameters));
-		params_copy.tranche_id = dsh_state->tranche;
-		ret = dshash_create(dsa, &params_copy, NULL);
+		PG_TRY();
+		{
+			memcpy(&params_copy, params, sizeof(dshash_parameters));
+			params_copy.tranche_id = dsh_state->tranche;
+			ret = dshash_create(dsa, &params_copy, NULL);
+		}
+		PG_CATCH();
+		{
+			dsa_detach(dsa);
+			PG_RE_THROW();
+		}
+		PG_END_TRY();
+
+		dsa_pin(dsa);
+		dsa_pin_mapping(dsa);
 
 		/* Store handles for other backends to use. */
 		dsh_state->dsa_handle = dsa_get_handle(dsa);
@@ -393,6 +468,32 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found)
 	else if (entry->type != DSMR_ENTRY_TYPE_DSH)
 		ereport(ERROR,
 				(errmsg("requested DSHash does not match type of existing entry")));
+	else if (entry->dsh.dsa_handle == DSA_HANDLE_INVALID)
+	{
+		NamedDSHState *dsh_state = &entry->dsh;
+		dshash_parameters params_copy;
+		dsa_area   *dsa;
+
+		dsa = dsa_create(dsh_state->tranche);
+
+		PG_TRY();
+		{
+			memcpy(&params_copy, params, sizeof(dshash_parameters));
+			params_copy.tranche_id = dsh_state->tranche;
+			ret = dshash_create(dsa, &params_copy, NULL);
+		}
+		PG_CATCH();
+		{
+			dsa_detach(dsa);
+			PG_RE_THROW();
+		}
+		PG_END_TRY();
+
+		dsa_pin(dsa);
+		dsa_pin_mapping(dsa);
+		dsh_state->dsa_handle = dsa_get_handle(dsa);
+		dsh_state->dsh_handle = dshash_get_hash_table_handle(ret);
+	}
 	else
 	{
 		NamedDSHState *dsh_state = &entry->dsh;
-- 
2.39.5 (Apple Git-154)


--RrQwG/d5bOC9SOJ1--