Skip to content

Properly handle shmget() ENOMEM error conditions on Windows - #9748

Open
cmb69 wants to merge 2 commits into
php:masterfrom
cmb69:cmb/shmget-enomem
Open

Properly handle shmget() ENOMEM error conditions on Windows#9748
cmb69 wants to merge 2 commits into
php:masterfrom
cmb69:cmb/shmget-enomem

Conversation

@cmb69

Copy link
Copy Markdown
Member

We need to properly handle MapViewOfFileEx() failures; otherwise strange error messages might be reported in the following. E.g. bug72858.phpt is likely to fail on 32bit Windows with "Warning: shm_attach(): Failed for key 0x64: File exists".


Note that this issue also affects older PHP versions, but the chance of MapViewOfFileEx() failing is very small there, because we only tried to map an info segment there, instead of trying to map the whole segment now, due to #8648.

Comment threadTSRM/tsrm_win32.c
@Lewiscowles1986

Copy link
Copy Markdown
Contributor

This looks like a really nice fix. Can I ask if there is any desire to get this merged?

Looking into newer PHP build failures on Windows, it does seem that they could be influenced by seemingly small bugs like this fixes.

@krakjoe
krakjoe changed the base branch from PHP-8.2 to masterAugust 31, 2025 19:39
@krakjoe

Copy link
Copy Markdown
Member

I'm unsure why no CI is running for this ? anyone ?

It looks correct and perfectly reasonable to merge, but I'm reluctant without running tests.

@cmb69 if you're around could you get this to run CI ?

@krakjoekrakjoe self-assigned this Aug 31, 2025
We need to properly handle `MapViewOfFileEx()` failures; otherwise
strange error messages might be reported in the following. E.g.
bug72858.phpt is likely to fail on 32bit Windows with "Warning:
shm_attach(): Failed for key 0x64: File exists".
@cmb69

Copy link
Copy Markdown
MemberAuthor

I have rebased onto current master, and now CI is running. Still not sure whether CI is really helpful here (as far as I know, we have only few tests regarding SHM on Windows). And I can barely remember what this PR is about.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@cmb69@Lewiscowles1986@krakjoe@iluuu1994