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

Started by Ayush Tiwariabout 24 hours ago3 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliessuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253859
psql -h localhost -U postgres

Built from patchset v1 (message #1), September 19, 2026 at 07:25 PM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t253859_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t253859_1 && git checkout t253859_1

Patchset v1 (message #1) is on t253859_1

Jump to latest
#1Ayush Tiwari
ayushtiwari.slg01@gmail.com

Hi,

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.)

The other one is a bit odd: ask for an existing area with
SHMEM_ATTACH_UNKNOWN_SIZE after startup in single-user mode, and we
tell you it "cannot be used during startup".

IIUC, the distinction we need here is whether we're still working out
the initial shmem requirements, not whether we have a postmaster.
0002 adds SRS_REQUESTING_AFTER_STARTUP for that. I couldn't see a clean
way to reuse SRS_REQUESTING without mixing those cases up.

I'm a bit on the fence about adding another state just for this.
With the PG19 release getting close, I thought I'd send this out for
feedback before spending more time iterating on it. Does the extra
state seem like the right approach?

Regards,
Ayush

Attachments:

t253859_1
v1-0001-Fix-legacy-shmem-reattachment-in-single-user-mode.patchapplication/octet-stream; name=v1-0001-Fix-legacy-shmem-reattachment-in-single-user-mode.patchDownload+35-16
v1-0002-Allow-unknown-size-shmem-attachments-in-single-user-mode.patchapplication/octet-stream; name=v1-0002-Allow-unknown-size-shmem-attachments-in-single-user-mode.patchDownload+67-9
#2Chao Li
li.evan.chao@gmail.com
In reply to: Ayush Tiwari (#1)
Re: [PATCH] Two remaining shmem attachment issues in single-user mode

On Sep 20, 2026, at 03:12, Ayush Tiwari <ayushtiwari.slg01@gmail.com> wrote:

Hi,

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.)

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 existing allocation and create one only if none exists. With the IsUnderPostmaster check, however, the postmaster always goes directly 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 whether the process is under the postmaster, even though I don't see why that distinction is needed here. In practice, the postmaster 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, and create one only if it doesn't exist.

The other one is a bit odd: ask for an existing area with
SHMEM_ATTACH_UNKNOWN_SIZE after startup in single-user mode, and we
tell you it "cannot be used during startup".

IIUC, the distinction we need here is whether we're still working out
the initial shmem requirements, not whether we have a postmaster.
0002 adds SRS_REQUESTING_AFTER_STARTUP for that. I couldn't see a clean
way to reuse SRS_REQUESTING without mixing those cases up.

I'm a bit on the fence about adding another state just for this.
With the PG19 release getting close, I thought I'd send this out for
feedback before spending more time iterating on it. Does the extra
state seem like the right approach?

I’m not very keen on adding a new state just to allow SHMEM_ATTACH_UNKNOWN_SIZE in single-user mode. While reading 0002, I had a few concerns:

1
```
-	if (IsUnderPostmaster)
+	if (IsUnderPostmaster || shmem_request_state == SRS_REQUESTING_AFTER_STARTUP)
 	{
 		if (options->size <= 0 && options->size != SHMEM_ATTACH_UNKNOWN_SIZE)
 			elog(ERROR, "invalid size %zd for shared memory request for \"%s\"",
```

Now that we have SRS_REQUESTING_AFTER_STARTUP, my first thought was whether the IsUnderPostmaster check could be removed entirely.

After digging into the code, I don't think so. 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.

That makes me wonder whether introducing another state is the simplest way to express this.

2
```
- *   DONE -> REQUESTING -> AFTER_STARTUP_ATTACH_OR_INIT -> DONE
+ *   DONE -> REQUESTING_AFTER_STARTUP -> AFTER_STARTUP_ATTACH_OR_INIT -> DONE
```

The main reason for introducing REQUESTING_AFTER_STARTUP seems to be to allow SHMEM_ATTACH_UNKNOWN_SIZE. But the next state is still AFTER_STARTUP_ATTACH_OR_INIT, so the new state is not really describing an attach-only phase.

That makes the state machine a little harder for me to reason about, because REQUESTING_AFTER_STARTUP is mainly distinguishing the context in which the request callback runs rather than representing a fundamentally different shared-memory lifecycle 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?

Regards,
Ayush
<v1-0001-Fix-legacy-shmem-reattachment-in-single-user-mode.patch><v1-0002-Allow-unknown-size-shmem-attachments-in-single-user-mode.patch>

Also, for v1-0001, I personally don't feel that the new test is really necessary. This doesn't seem like particularly fragile logic that's likely to regress accidentally, and I've noticed that Tom generally seems reluctant to increase test runtime, even by a very small amount, unless the test provides clear value.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

#3Ayush Tiwari
ayushtiwari.slg01@gmail.com
In reply to: Chao Li (#2)
Re: [PATCH] Two remaining shmem attachment issues in single-user mode

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 existing allocation and create one only if none exists. With the IsUnderPostmaster check, however, the postmaster always goes directly 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 whether the process is under the postmaster, even though I don't see why that distinction is needed here. In practice, the postmaster 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, and create 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