[#47247] - [5.4] Fix boolean OR to bitwise OR for pcntl_waitpid flags in DaemonApplication
- Pending
- Medium
- Build: 5.4-dev
- # 47247
- Diff
- D3S-Gaurav:fix/daemon-boolean-to-bitwise-or
User tests: Successful: Unsuccessful:
- Fix boolean OR (||) to bitwise OR (|) for pcntl_waitpid flags in SIGCHLD handler
Pull Request resolves # .
- I read the Generative AI policy and my contribution is either not created with the help of AI or is compatible with the policy and GNU/GPL 2 or later.
Summary of Changes
The SIGCHLD signal handler in DaemonApplication::signal() used WNOHANG || WUNTRACED (boolean OR) instead of WNOHANG | WUNTRACED (bitwise OR).
WNOHANG= 1,WUNTRACED= 2WNOHANG || WUNTRACED→true→ cast to1(onlyWNOHANG)WNOHANG | WUNTRACED→3(both flags correctly combined)
The boolean OR silently drops the WUNTRACED flag, meaning stopped (but not terminated) child processes are not reaped by pcntl_waitpid().
Testing Instructions
- Review the single-character change (
||→|). - Confirm
WNOHANG | WUNTRACEDis the correct flag combination forpcntl_waitpid()per PHP docs.
Actual result BEFORE applying this Pull Request
WUNTRACED flag is silently dropped — stopped child processes are not reaped.
Expected result AFTER applying this Pull Request
Both WNOHANG and WUNTRACED flags are correctly passed to pcntl_waitpid().
Link to documentations
Please select:
-
Documentation link for guide.joomla.org:
-
No documentation changes for guide.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
From reading the code and the PHP doc, the change seems to be right.
However, maybe the mistake of the current code (without this PR) resulted (by mistake) in the right behaviour, and fixing it causes an unwanted behaviour change?
Or in other words: Maybe the right fix would be to remove the || WUNTRACED so we keep existing behaviour?
To summarise: It needs real tests, code review is not sufficient.
hi @brianteeman ,
I use AI tools generally to help me understand large codebases faster to gett a high-level picture of how different parts fit together. I was actually looking into how Joomla-cms handles the Content History versioning system for a test fix I'd been working on (#47200), and while tracing how the application lifecycle works, I got in DaemonApplication.php to understand the signal handling flow. The || vs | in the pcntl_waitpid() call i remembered it because I hit the exact same issue before when I was writing a small queue worker in PHP that forked child processes, and spent a frustrating couple of hours debugging why stopped children weren't being picked up by the parent. Turned out I'd used || instead of | when combining WNOHANG and WUNTRACED.
Hi @richard67 ,
That’s a good point. From what I can see, the current behaviour works because child processes normally exit rather than being stopped, so the missing WUNTRACED likely hasn’t surfaced in practice. My understanding is that the change mainly restores the intended flag usage rather than changing the common execution path, but I think i should dig deeper if there are scenarios where the existing behaviour might be relied on.
thanks for confirming that this PR is ai generated - i wont waste my time
How did you find this?