Implement pcntl_waitid

[vrza]

Closes #14616

To round out the support for the "wait" family of functions this PR adds the pcntl_waitid function.

Here's the POSIX reference to waitid:

https://pubs.opengroup.org/onlinepubs/9699919799/functions/waitid.html

The waitid() function shall obtain status information pertaining to termination, stop, and/or continue events in one of the caller's child processes

Since status information struct is shared with sigwaitinfo, relevant code from pcntl_sigwaitinfo has been reused.

Note to maintainers: if this patch is accepted, I'll be happy to provide the English documentation in the follow-up PR.

[devnexen]

Note: you can perfectly add non standard flags but you need to make sure they exist before defining them

#ifdef P_PIDFD
/**
 * @var int
 * @cvalue LONG_CONST(P_PIDFD)
 */
const P_PIDFD = UNKNOWN;
#endif

FreeBSD has its own non standard flags if you want to add them too.

Once you fixed all, we need a test for this.

[vrza]

@devnexen Thanks for the quick review! I addressed your comments (ValueErrors, unit test, guard for the Linux speciifc P_PIDFD), please take another look.

[devnexen]

I see the progress but take your time :) I ll have a look later.

[devnexen]

starting to look good, but I ll let it rest enough days to give a chance to other reviewers :).

[vrza]

Please review this patch, it is done and tested to work on:

• GNU/Linux
• OpenBSD
• NetBSD
• FreeBSD
• DragonFly BSD
• macOS [1]
• Illumos

Error handling is done in POSIX style, by accessing errno through pcntl_get_last_error, and (already existing) constants PCNTL_ECHILD, PCNTL_EINTR and PCNTL_EINVAL, that can also be passed to pcntl_strerror to obtain an error message. This is in line with how most other POSIX wrapper functions in PHP are implemented, in particular the implementation is consistent with close relatives pcntl_wait and pcntl_waitpid which I reviewed and found to have satisfactory API design.

[1] On macOS, though, existence of waitid(2) is undocumented, but it seems to work for the POSIX-compliant tested use cases.

[devnexen]

The FreeBSD failing test seems legit.

[devnexen]

@petk what do you think build system wise ?

[vrza]

The FreeBSD failing test seems legit.

@devnexen Right, there was a race condition in the test. Fixed it, thanks for noticing and raising.

On a related note, in multiple other unit tests there are guards that skip them on FreeBSD. I suspect many of them could also be due to incorrect (buggy) test code.

For example, there is an issue in pcntl/tests/002.phpt, that was silenced by Nikita Popov in 2021:

74859783398 (Nikita Popov       2021-06-11 11:57:42 +0200 15) elseif (str_contains(PHP_OS, 'FreeBSD')) die('skip Results in parallel test runner hang on FreeBSD');

I took a quick look at that test code, looks like a race condition (SIGTERM can be sent too early). I could fix that one as well, but as a separate issue/PR.

[petk]

@petk what do you think build system wise ?

Build system changes look very good and professional. Even nicely checked those HAVE_DECL_ ones with #if defined 🤝 Except maybe those two #include arguments can be wraped into quotes without new lines in them: [#include <sys/wait.h>]

[vrza]

@petk Thanks for reviewing 🤝 The aim is to be strict and explicit about requirements, but flexible about the way they are provisioned.

Regarding m4 formatting... Starting with a disclaimer that this is purely a cosmetic discussion with no impact to functionality... Some of those m4 lines are very long, I'd like to break them up somewhere for readability, popping the includes out seemed like a nice place to break the line, although the list of declarations is also rather long. Eventually includes turned out to be a short, unprotected, one-liner.

Perhaps a compromise, with Lisp-style trailing braces:

  AC_CHECK_FUNCS([WIFCONTINUED],,
    [AC_CHECK_DECL([WIFCONTINUED], [AC_DEFINE([HAVE_WIFCONTINUED], [1])],,
      [#include <sys/wait.h>])])

  AC_CHECK_DECLS([WCONTINUED, WEXITED, WSTOPPED, WNOWAIT, P_ALL, P_PIDFD, P_UID, P_JAILID],,,
    [#include <sys/wait.h>])

[petk]

@petk Thanks for reviewing 🤝 The aim is to be strict and explicit about requirements, but flexible about the way they are provisioned.

Regarding m4 formatting... Starting with a disclaimer that this is purely a cosmetic discussion with no impact to functionality... Some of those m4 lines are very long, I'd like to break them up somewhere for readability, popping the includes out seemed like a nice place to break the line, although the list of declarations is also rather long. Eventually includes turned out to be a short, unprotected, one-liner.

Perhaps a compromise, with Lisp-style trailing braces:

  AC_CHECK_FUNCS([WIFCONTINUED],,
    [AC_CHECK_DECL([WIFCONTINUED], [AC_DEFINE([HAVE_WIFCONTINUED], [1])],,
      [#include <sys/wait.h>])])

  AC_CHECK_DECLS([WCONTINUED, WEXITED, WSTOPPED, WNOWAIT, P_ALL, P_PIDFD, P_UID, P_JAILID],,,
    [#include <sys/wait.h>])

Perfect. Yes that's how I was thinking also. :D

[devnexen]

Looking ok by me, cc @kocsismate

[vrza]

While this is in review I started working on documentation.

[bukka]

Looks reasonable

[devnexen]

Since it targets master, we can always improve/fix upon if needs be. I ll merge it in few days if no disagreement arise in the meantime.