Skip to content

Prevents tests from leaking System V IPC objects - #22912

Open
NickSdot wants to merge 2 commits into
php:masterfrom
NickSdot:fix/test-ipc-probe-cleanup
Open

Prevents tests from leaking System V IPC objects #22912
NickSdot wants to merge 2 commits into
php:masterfrom
NickSdot:fix/test-ipc-probe-cleanup

Conversation

@NickSdot

Copy link
Copy Markdown
Contributor

The two tests leave System V IPC objects behind. Repeated test-suite runs can exhaust platform IPC limits and cause unrelated SysV tests to fail. Happened on macOS where the message queue limit is relatively low.

1) arginfo_zpp_mismatch.phpt
Calls every internal function with 0..8 null arguments. Means, 4 System V IPC functions create 3 queues, 3 shared-memory segments, and 4 semaphore sets per run -- these survive the process that created them.

The 4 affected functions are now handled early with a helper. The argument counts stay the same, but sem_get() and shm_attach() use 0600 for their permission argument, because null becomes 0000 which can create non-removable objects.

2) gh16592.phpt
created a message queue without removing it; now uses a private queue and cleans up after it.

@NickSdot

NickSdot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@iliaal since this one is related to the other one you just looked at, please allow me to ping you here (no code owner here, too).

Comment thread ext/sysvmsg/tests/gh16592.phpt Outdated
@iliaal

iliaal commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Checked this locally on a build with the three sysv extensions. Unpatched, arginfo_zpp_mismatch.phpt leaves 3 queues, 4 semaphore sets and 3 segments behind per run; patched it leaves none, with the expected output unchanged. The 0..8 coverage is intact since the arginfo/zpp check keys off argument counts rather than values. The strict variant needs nothing because strict_types=1 turns the null into a TypeError before the C function runs.

Two other tests leak the same way, if you want them in scope. ext/shmop/tests/shmop_open_private.phpt opens two IPC_PRIVATE segments and never calls shmop_delete(), so it grows by 2 per run (I measured 2, 4, 6). ext/sysvshm/tests/gh16591.phpt does shm_attach(1) with only a shm_detach(), same shape as the gh16592 fix here, bounded at one segment.

Both of your workarounds are there because of extension bugs that deserve their own PR. msg_queue_exists() is RETURN_BOOL(msgget(key, 0) >= 0) and msgget(IPC_PRIVATE, 0) always creates, so msg_queue_exists(0) creates a queue and discards the id. shm_attach() orphans the segment it created when shmat() fails; the failed shmget(key, 0, 0) probe is the signal that it was ours, so an IPC_RMID there is available if gated on that.

@NickSdot
NickSdot force-pushed the fix/test-ipc-probe-cleanup branch from ab7526b to 07a5e09 Compare July 30, 2026 06:26
@NickSdot

Copy link
Copy Markdown
Contributor Author

Two other tests leak the same way, if you want them in scope. ext/shmop/tests/shmop_open_private.phpt opens two IPC_PRIVATE segments and never calls shmop_delete(), so it grows by 2 per run (I measured 2, 4, 6). ext/sysvshm/tests/gh16591.phpt does shm_attach(1) with only a shm_detach(), same shape as the gh16592 fix here, bounded at one segment.

These are the two that were fixed in #22911.

Both of your workarounds are there because of extension bugs that deserve their own PR. msg_queue_exists() is RETURN_BOOL(msgget(key, 0) >= 0) and msgget(IPC_PRIVATE, 0) always creates, so msg_queue_exists(0) creates a queue and discards the id. shm_attach() orphans the segment it created when shmat() fails; the failed shmget(key, 0, 0) probe is the signal that it was ours, so an IPC_RMID there is available if gated on that.

I had Codex look at it. The msg_queue_exists change seems straight forward; I get it. The shm_attach change also looks fairly minimal: in shmget(shm_key, 0, 0) we know we created it, so we can flag it as owned by us, then when shmat() fails we can clean up what we own. Is that what you had in mind? Though, not sure if something is missing. Unless you are keen to handhold me through it if something is not right, I'd rather not open an PR for it myself. What do you think?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants