On Mon, Nov 10, 2025 at 05:17:47PM -0600, Sami Imseih wrote:
> Maybe we can improve the error message by showing
> the "name", like this, In case, there are nested calls.
> 
> ```
> else if (!entry->initialized)
> {
> ereport(ERROR,
> (errmsg("requested DSM segment \"%s\" failed initialization", name)));
> }
> ```

Done.

-- 
nathan
>From 0b12db92e17dd315cc08b5a918196b685e3f6f61 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Mon, 10 Nov 2025 11:43:51 -0600
Subject: [PATCH v3 1/2] DSM registry: ERROR if entry was not initialized.

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

diff --git a/src/backend/storage/ipc/dsm_registry.c 
b/src/backend/storage/ipc/dsm_registry.c
index a926b9c3f32..7eba8a8cffb 100644
--- a/src/backend/storage/ipc/dsm_registry.c
+++ b/src/backend/storage/ipc/dsm_registry.c
@@ -93,6 +93,7 @@ typedef struct DSMRegistryEntry
 {
        char            name[NAMEDATALEN];
        DSMREntryType type;
+       bool            initialized;
        union
        {
                NamedDSMState dsm;
@@ -216,6 +217,7 @@ 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);
@@ -228,13 +230,21 @@ 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 does not match 
type of existing entry")));
+                               (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)));
        else if (entry->dsm.size != size)
                ereport(ERROR,
-                               (errmsg("requested DSM segment size does not 
match size of existing segment")));
+                               (errmsg("requested DSM segment \"%s\" does not 
match size of existing entry",
+                                               name)));
        else
        {
                NamedDSMState *state = &entry->dsm;
@@ -297,6 +307,7 @@ 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);
@@ -308,10 +319,17 @@ 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 does not match type of 
existing entry")));
+                               (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)));
        else
        {
                NamedDSAState *state = &entry->dsa;
@@ -372,6 +390,7 @@ 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);
@@ -389,10 +408,17 @@ 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 does not match type 
of existing entry")));
+                               (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)));
        else
        {
                NamedDSHState *dsh_state = &entry->dsh;
-- 
2.39.5 (Apple Git-154)

>From bc876604668340706ead2dc64db2b692f0658618 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Mon, 10 Nov 2025 12:03:47 -0600
Subject: [PATCH v3 2/2] test_dsa: Avoid leaking LWLock tranches.

---
 src/test/modules/test_dsa/test_dsa.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/test/modules/test_dsa/test_dsa.c 
b/src/test/modules/test_dsa/test_dsa.c
index 01d5c6fa67f..21e4ce6a745 100644
--- a/src/test/modules/test_dsa/test_dsa.c
+++ b/src/test/modules/test_dsa/test_dsa.c
@@ -13,25 +13,35 @@
 #include "postgres.h"
 
 #include "fmgr.h"
+#include "storage/dsm_registry.h"
 #include "storage/lwlock.h"
 #include "utils/dsa.h"
 #include "utils/resowner.h"
 
 PG_MODULE_MAGIC;
 
+static void
+init_tranche(void *ptr)
+{
+       int                *tranche_id = (int *) ptr;
+
+       *tranche_id = LWLockNewTrancheId("test_dsa");
+}
+
 /* Test basic DSA functionality */
 PG_FUNCTION_INFO_V1(test_dsa_basic);
 Datum
 test_dsa_basic(PG_FUNCTION_ARGS)
 {
-       int                     tranche_id;
+       int                *tranche_id;
+       bool            found;
        dsa_area   *a;
        dsa_pointer p[100];
 
-       /* XXX: this tranche is leaked */
-       tranche_id = LWLockNewTrancheId("test_dsa");
+       tranche_id = GetNamedDSMSegment("test_dsa", sizeof(int),
+                                                                       
init_tranche, &found);
 
-       a = dsa_create(tranche_id);
+       a = dsa_create(*tranche_id);
        for (int i = 0; i < 100; i++)
        {
                p[i] = dsa_allocate(a, 1000);
@@ -62,17 +72,18 @@ PG_FUNCTION_INFO_V1(test_dsa_resowners);
 Datum
 test_dsa_resowners(PG_FUNCTION_ARGS)
 {
-       int                     tranche_id;
+       int                *tranche_id;
+       bool            found;
        dsa_area   *a;
        dsa_pointer p[10000];
        ResourceOwner oldowner;
        ResourceOwner childowner;
 
-       /* XXX: this tranche is leaked */
-       tranche_id = LWLockNewTrancheId("test_dsa");
+       tranche_id = GetNamedDSMSegment("test_dsa", sizeof(int),
+                                                                       
init_tranche, &found);
 
        /* Create DSA in parent resource owner */
-       a = dsa_create(tranche_id);
+       a = dsa_create(*tranche_id);
 
        /*
         * Switch to child resource owner, and do a bunch of allocations in the
-- 
2.39.5 (Apple Git-154)

Reply via email to