Re: [PATCH] Two remaining shmem attachment issues in single-user mode - Mailing list pgsql-hackers

Hi,

On Sun, 20 Sept 2026 at 13:58, Chao Li <li.evan.chao@gmail.com> wrote:
> > 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.)
>
> I think it’s reasonable to remove the IsUnderPostmaster check for two reasons:
>
> * IsUnderPostmaster is always false in single-user mode, so the current check prevents reattachment there.
> * The current logic feels unnecessarily asymmetric. The natural pattern for ShmemInitStruct() is to first look for an
existingallocation and create one only if none exists. With the IsUnderPostmaster check, however, the postmaster always
goesdirectly to creation, while a child process first looks for an existing allocation and creates one if it is not
found.That makes the code a little confusing to reason about. It suggests that the create-or-attach behavior depends on
whetherthe process is under the postmaster, even though I don't see why that distinction is needed here. In practice,
thepostmaster initializes shared memory before child processes are started anyway. 
>
> So, removing the check makes the behavior simpler and more consistent, always look for an existing allocation first,
andcreate one only if it doesn't exist. 

Thanks for taking a look.

> IsUnderPostmaster is still needed for cases such as EXEC_BACKEND
> children, so the new state seems to be added specifically to cover
> the single-user after-startup case.

Yes, I think we'd still need that allowance for EXEC_BACKEND. The new
state would be used for late requests in normal backends too, though
it's the standalone case where it changes what we accept.

> But the next state is still AFTER_STARTUP_ATTACH_OR_INIT, so the new
> state is not really describing an attach-only phase.

I was thinking of it as distinguishing when the request happens,
rather than making it attach-only. A late request with a known size
could still allocate a new area; an unknown-size request would need
to find an existing one. Perhaps the comment could make that clearer?

I'm not wedded to another enum value here. I just couldn't see how to
keep the startup and late-request checks separate with the existing
state.

> If the only additional case we need to distinguish here is a late
> request in single-user mode, would it be simpler to check
> MyBackendType == B_STANDALONE_BACKEND instead of adding a new request
> state?

AFAICS, InitStandaloneProcess() sets B_STANDALONE_BACKEND before we
get to ShmemCallRequestCallbacks(). Wouldn't that also let an
unknown-size request through during standalone startup, when we're
still working out how much memory to allocate?

That's the distinction I was trying to preserve in 0002. I may be
missing another condition we could use alongside the backend type,
though.

> Also, for v1-0001, I personally don't feel that the new test is really
> necessary.

Fair point about the extra startup. I'd lean towards keeping a small
reproducer, since the existing tests hadn't caught the double-call
case, but perhaps a separate invocation is unnecessary.

Would folding the legacy check into the existing single-user run be
a reasonable compromise? With exit_on_error already enabled, adding
the query there should let us cover it without another backend startup.
[I haven't yet tried it though]

Regards,
Ayush