From 8ef5c0f197f0a095a19b4642ad94c56422e40d40 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Sun, 12 Nov 2023 23:10:28 +0100 Subject: [PATCH 2/3] Fix dsa.c with different resource owners. The comments in dsa.c suggested that areas were owned by resource owners, but it was not in fact tracked explicitly. The DSM attachments held by the dsa were owned by resource owners, but not the area itself. That led to confusion if you used one resource owner to attach or create the area, but then switched to a different resource owner when allocating or even just accessing the allocations in the area with dsa_get_address(). The additional DSM segments associated with the area would get owned by a different resource owner than the initial segment. To fix, add an explicit 'resowner' field to dsa_area. It replaces the 'mapping_pinned' flag; resowner == NULL now indicates that the mapping is pinned. This fixes the bug demonstrated by the test case from previous commit. Reviewed-by: Alexander Lakhin Discussion: https://www.postgresql.org/message-id/11b70743-c5f3-3910-8e5b-dd6c115ff829%40gmail.com --- src/backend/utils/mmgr/dsa.c | 38 ++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/src/backend/utils/mmgr/dsa.c b/src/backend/utils/mmgr/dsa.c index 8d1aace40ac..71cf11b1894 100644 --- a/src/backend/utils/mmgr/dsa.c +++ b/src/backend/utils/mmgr/dsa.c @@ -59,6 +59,7 @@ #include "utils/dsa.h" #include "utils/freepage.h" #include "utils/memutils.h" +#include "utils/resowner.h" /* * The size of the initial DSM segment that backs a dsa_area created by @@ -368,8 +369,13 @@ struct dsa_area /* Pointer to the control object in shared memory. */ dsa_area_control *control; - /* Has the mapping been pinned? */ - bool mapping_pinned; + /* + * All the mappings are owned by this. The dsa_area itself is not + * directly tracked by the ResourceOwner, but the effect is the same. + * NULL if the attachment has session lifespan, i.e if dsa_pin_mapping() + * has been called. + */ + ResourceOwner resowner; /* * This backend's array of segment maps, ordered by segment index @@ -645,12 +651,14 @@ dsa_pin_mapping(dsa_area *area) { int i; - Assert(!area->mapping_pinned); - area->mapping_pinned = true; + if (area->resowner != NULL) + { + area->resowner = NULL; - for (i = 0; i <= area->high_segment_index; ++i) - if (area->segment_maps[i].segment != NULL) - dsm_pin_mapping(area->segment_maps[i].segment); + for (i = 0; i <= area->high_segment_index; ++i) + if (area->segment_maps[i].segment != NULL) + dsm_pin_mapping(area->segment_maps[i].segment); + } } /* @@ -1264,7 +1272,7 @@ create_internal(void *place, size_t size, */ area = palloc(sizeof(dsa_area)); area->control = control; - area->mapping_pinned = false; + area->resowner = CurrentResourceOwner; memset(area->segment_maps, 0, sizeof(dsa_segment_map) * DSA_MAX_SEGMENTS); area->high_segment_index = 0; area->freed_segment_counter = 0; @@ -1320,7 +1328,7 @@ attach_internal(void *place, dsm_segment *segment, dsa_handle handle) /* Build the backend-local area object. */ area = palloc(sizeof(dsa_area)); area->control = control; - area->mapping_pinned = false; + area->resowner = CurrentResourceOwner; memset(&area->segment_maps[0], 0, sizeof(dsa_segment_map) * DSA_MAX_SEGMENTS); area->high_segment_index = 0; @@ -1743,6 +1751,7 @@ get_segment_by_index(dsa_area *area, dsa_segment_index index) dsm_handle handle; dsm_segment *segment; dsa_segment_map *segment_map; + ResourceOwner oldowner; /* * If we are reached by dsa_free or dsa_get_address, there must be at @@ -1761,11 +1770,12 @@ get_segment_by_index(dsa_area *area, dsa_segment_index index) elog(ERROR, "dsa_area could not attach to a segment that has been freed"); + oldowner = CurrentResourceOwner; + CurrentResourceOwner = area->resowner; segment = dsm_attach(handle); + CurrentResourceOwner = oldowner; if (segment == NULL) elog(ERROR, "dsa_area could not attach to segment"); - if (area->mapping_pinned) - dsm_pin_mapping(segment); segment_map = &area->segment_maps[index]; segment_map->segment = segment; segment_map->mapped_address = dsm_segment_address(segment); @@ -2067,6 +2077,7 @@ make_new_segment(dsa_area *area, size_t requested_pages) size_t usable_pages; dsa_segment_map *segment_map; dsm_segment *segment; + ResourceOwner oldowner; Assert(LWLockHeldByMe(DSA_AREA_LOCK(area))); @@ -2151,12 +2162,13 @@ make_new_segment(dsa_area *area, size_t requested_pages) } /* Create the segment. */ + oldowner = CurrentResourceOwner; + CurrentResourceOwner = area->resowner; segment = dsm_create(total_size, 0); + CurrentResourceOwner = oldowner; if (segment == NULL) return NULL; dsm_pin_segment(segment); - if (area->mapping_pinned) - dsm_pin_mapping(segment); /* Store the handle in shared memory to be found by index. */ area->control->segment_handles[new_index] = -- 2.39.2