Re: [PATCH] Two remaining shmem attachment issues in single-user mode

On 24/09/2026 16:07, Ashutosh Bapat wrote:
> On Wed, Sep 23, 2026 at 6:50 PM Ayush Tiwari
> <ayushtiwari(dot)slg01(at)gmail(dot)com> wrote:
>>
>> On Wed, 23 Sept 2026 at 17:53, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>>>
>>> On 19/09/2026 22:12, Ayush Tiwari wrote:
>>>>
>>>> I found two more shmem attachment issues in single-user mode after
>>>> the recent fixes. Patches attached.
>>>>
>>>> With ShmemInitStruct(), a second call for the same name and size errors
>>>> out with "already initialized". We only look for the old allocation if
>>>> IsUnderPostmaster is true, so a standalone backend goes straight down
>>>> the allocation path again.
>>>>
>>>> 0001 drops that condition. It fixes postmaster-startup reattachment
>>>> too, which I did hesitate over at first. But AFAICS that was supported
>>>> before the refactoring, and it's what the legacy API still promises.
>>>> So I'd lean towards restoring that behaviour in both places. Is there
>>>> a reason not to? (The size and initialization checks are still there.)
>>>
>>> Hmm, so the scenario is:
>>>
>>> 1. Start postgres in single-user mode
>>> 2. Load an extension that calls ShmemInitStruct() to alloc a shmem area
>>> 3. The extension calls ShmemInitStruct() again, to get a pointer to the
>>> already-initialized area.
>>>
>>> I didn't think of that scenario. There's no reason to not support that,
>>> although it's pretty weird for an extension to do that. One use case
>>> might be to have a shared struct between two cooperating extensions, so
>>> that they both call ShmemInitStruct() to get a pointer to the same area.
>>> I'm not aware of any extensions actually doing that, though.
>>>
>>> I'll commit that fix, thanks!
>
> The code changes look good to me. There are opportunities to make the
> test less confusing and cover more scenarios.

Thanks for the review, I committed this part now.

> When attaching to existing shared memory through the callback-based
> API after startup, the supplied attach callback is invoked. In
> contrast, ShmemInitStruct() does not invoke an attach callback. People
> using both the legacy and new APIs may find this difference
> surprising. Should we document it explicitly in the ShmemInitStruct()
> documentation in a separate patch?

Hmm, I don't quite understand. ShmemInitStruct() didn't have an attach
callback at all, so I don't know what there is to document here.

> I think there's also some value in porting parts of the
> 001_late_shmem_alloc test to the PostgreSQL 18 stable branch to test
> and document the legacy behaviour. The code has changed so much that
> it's hard to know what the legacy behaviour was. Even porting the
> whole test and adjusting it for the APIs there might be worth it. That
> way we can easily compare the new behaviour with the legacy behaviour
> simply by looking at the differences in the test. This could be a
> separate patch.

Yeah, perhaps. For now, I decided to leave out this
"test_shmem_legacy()" test.

(But thanks, Ayush, for including it! As Jacob said earlier, it's really
useful to have reproducers for every issue, even though they're not all
committed to the repository.)

> + $query = "SELECT test_shmem_legacy();\n";
> + $result = run_log(\(at)command, '<' => \$query);
> + ok($result, "legacy shared memory reattachment works in single-user mode");
>
> I think we should add this test to the shared_preload_libraries
> section as well to check that legacy shared memory reattachment works
> correctly when the module is loaded through that GUC. Over time, we
> may fail to maintain compatibility with the legacy APIs and won't
> notice the drift without tests covering those cases.

Fair. My goal was that ShmemInitStruct() would be just a thin wrapper
around the same functions that the new API uses, so that there's little
chance of breaking just the legacy function. But I'm not sure I managed
to make it quite so thin...

- Heikki