On Mon, Nov 13, 2023 at 11:16 AM Heikki Linnakangas <hlinn...@iki.fi> wrote: > On 11/11/2023 14:00, Alexander Lakhin wrote: > > 10.11.2023 17:26, Heikki Linnakangas wrote: > >> I think that is surprising behavior from the DSA facility. When you make > >> allocations with dsa_allocate() or just call > >> dsa_get_address() on an existing dsa_pointer, you wouldn't expect the > >> current resource owner to matter. I think > >> dsa_create/attach() should store the current resource owner in the > >> dsa_area, for use in subsequent operations on the > >> DSA, per attached patch > >> (0002-Fix-dsa.c-with-different-resource-owners.patch).
Yeah, I agree that it is surprising and dangerous that the DSM segments can have different owners if you're not careful, and it's not clear that you have to be. Interesting that no one ever reported breakage in parallel query due to this thinko, presumably because the current resource owner always turns out to be either the same one or something with longer life. > > With the patch 0002 applied, I'm observing another anomaly: > > Ok, so the ownership of a dsa was still muddled :-(. Attached is a new > version that goes a little further. It replaces the whole > 'mapping_pinned' flag in dsa_area with the 'resowner'. When a mapping is > pinned, it means that 'resowner == NULL'. This is now similar to how the > resowner field and pinning works in dsm.c. This patch makes sense to me. It might be tempting to delete the dsa_pin_mapping() interface completely and say that if CurrentResourceOwner == NULL, then it's effectively (what we used to call) pinned, but I think it's useful for exception-safe construction of multiple objects to be able to start out with a resource owner and then later 'commit' by clearing it. As seen in GetSessionDsmHandle(). I don't love the way this stuff is not very explicit, and if we're going to keep doing this 'dynamic scope' stuff then I wish we could find a scoping notation that looks more like the stuff one sees in other languages that say something like "with-resource-owner(area->resowner) { block of code }".