Skip to content

Implement pcntl_waitid #14617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 44 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
1fe4de1
ext/pcntl: Added new function pcntl_waitid
vrza Jun 20, 2024
0483f22
Validate flags, fill siginfo array
vrza Jun 20, 2024
ecefc18
Expose needed constants from wait.h
vrza Jun 20, 2024
c8ff3b6
Add ifdef guards around WEXITED, WSTOPPED, WNOWAIT
vrza Jun 20, 2024
2dd62b9
Unit tests for pcntl_waitid
vrza Jun 20, 2024
0ccd92f
Throw a ValueError if parameter validation fails
vrza Jun 20, 2024
712b536
Evaluate in numerical order
vrza Jun 20, 2024
dfe3bfe
Guard Linux-specific P_PIDFD constant
vrza Jun 20, 2024
e23bc2d
Linux-specific iptype validation
vrza Jun 20, 2024
651df1f
Include linux/wait.h on Linux to check for idtype constants
vrza Jun 20, 2024
3ab725d
Leave idtype and flags validation out as it is platform-specific
vrza Jun 20, 2024
93f4fad
Added FreeBSD specific idtypes
vrza Jun 20, 2024
ee6b65d
Added NetBSD specific idtypes
vrza Jun 20, 2024
4830687
Added check for linux/wait.h
vrza Jun 20, 2024
e3cfd08
Guard pcntl_waitid behind HAVE_WAITID
vrza Jun 20, 2024
ba5d347
Test pcntl_waitid failing due to invalid arguments
vrza Jun 20, 2024
324b79c
Pick a better bad value for idtype in test
vrza Jun 20, 2024
ab07bc0
Removed unnecessary comments
vrza Jun 20, 2024
f01aeb3
Comment on NetBSD specific idtypes
vrza Jun 20, 2024
52fce3d
Renamed variable success to status
vrza Jun 21, 2024
d39033a
Made the id param to waitid nullable
vrza Jun 21, 2024
5deaba1
Expect warnings about invalid arguments
vrza Jun 21, 2024
e1399cf
More tests, including testing error messages
vrza Jun 21, 2024
d173be1
Implemented weak idtype validation
vrza Jun 21, 2024
5793dcd
Added test for weak idtype validation
vrza Jun 21, 2024
509c92f
Added tests with PHP_INT_MAX
vrza Jun 21, 2024
31f3856
Throw ValueError on errno == EINVAL
vrza Jun 21, 2024
a0df466
Added test for null id when idtype != P_ALL
vrza Jun 22, 2024
8763548
Moved idtype constants detection to autoconf
vrza Jun 23, 2024
5021e88
Fixed typo in comment
vrza Jun 23, 2024
1bf8873
FreeBSD fixes
vrza Jun 23, 2024
25fc4db
macOS fills in siginfo_t even on no state change
vrza Jun 23, 2024
2df267a
Changed API to closely match wait and waitpid
vrza Jun 23, 2024
9124b1e
AC_CHECK_DECLS for WEXITED, WSTOPPED, WNOWAIT
vrza Jun 23, 2024
80a1da8
Test pcntl_get_last_error()
vrza Jun 24, 2024
ea33122
AC_CHECK_DECLS superseded importing linux/wait.h
vrza Jun 24, 2024
5f540d8
Improved m4 code intentation
vrza Jun 24, 2024
595b9c4
Better default params
vrza Jun 25, 2024
0011541
Fixed bad value for zend long
vrza Jun 25, 2024
1993ec1
Stricter preprocessor guards
vrza Jun 25, 2024
19e0612
Updated NEWS
vrza Jun 26, 2024
207ea17
Fixed race condition in unit tests
vrza Jun 27, 2024
e69f6c7
Marked unused param with underscore
vrza Jun 27, 2024
ed38397
m4 formatting
vrza Jun 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ PHP NEWS
. Added pcntl_getqos_class/pcntl_setqos_class for macOs. (David Carlier)
. Added SIGCKPT/SIGCKPTEXIT constants for DragonFlyBSD. (David Carlier)
. Added FreeBSD's SIGTRAP handling to pcntl_siginfo_to_zval. (David Carlier)
. Added POSIX pcntl_waitid. (Vladimir Vrzić)

- PCRE:
. Upgrade bundled pcre2lib to version 10.43. (nielsdos)
Expand Down
8 changes: 8 additions & 0 deletions ext/pcntl/config.m4
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,16 @@ if test "$PHP_PCNTL" != "no"; then
unshare
wait3
wait4
waitid
]))

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

dnl if unsupported, -1 means automatically ENOSYS in this context
AC_CACHE_CHECK([if sched_getcpu is supported], [php_cv_func_sched_getcpu],
[AC_RUN_IFELSE([AC_LANG_SOURCE([
Expand Down
52 changes: 50 additions & 2 deletions ext/pcntl/pcntl.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

#include "php.h"
#include "ext/standard/info.h"
#include "php_pcntl.h"
#include "php_signal.h"
#include "php_ticks.h"
#include "zend_fibers.h"
Expand All @@ -40,6 +39,22 @@
#include <sys/resource.h>
#endif

#ifdef HAVE_WAITID
#if defined (HAVE_DECL_P_ALL) && HAVE_DECL_P_ALL == 1
#define HAVE_POSIX_IDTYPES 1
#endif
#if defined (HAVE_DECL_P_PIDFD) && HAVE_DECL_P_PIDFD == 1
#define HAVE_LINUX_IDTYPES 1
#endif
#if defined (HAVE_DECL_P_UID) && HAVE_DECL_P_UID == 1
#define HAVE_NETBSD_IDTYPES 1
#endif
#if defined (HAVE_DECL_P_JAILID) && HAVE_DECL_P_JAILID == 1
#define HAVE_FREEBSD_IDTYPES 1
#endif
#endif

#include "php_pcntl.h"
#include <errno.h>
#if defined(HAVE_UNSHARE) || defined(HAVE_SCHED_SETAFFINITY) || defined(HAVE_SCHED_GETCPU)
#include <sched.h>
Expand Down Expand Up @@ -385,6 +400,39 @@ PHP_FUNCTION(pcntl_waitpid)
}
/* }}} */

#if defined (HAVE_WAITID) && defined (HAVE_POSIX_IDTYPES) && defined (HAVE_DECL_WEXITED) && HAVE_DECL_WEXITED == 1
PHP_FUNCTION(pcntl_waitid)
{
zend_long idtype = P_ALL;
zend_long id = 0;
bool id_is_null = 1;
zval *user_siginfo = NULL;
zend_long options = WEXITED;

ZEND_PARSE_PARAMETERS_START(0, 4)
Z_PARAM_OPTIONAL
Z_PARAM_LONG(idtype)
Z_PARAM_LONG_OR_NULL(id, id_is_null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this parameter nullable? Apparently, it's not used anywhere.

Copy link
Contributor Author

@vrza vrza Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a minor style point. From the API design standpoint it's not strictly necessary to allow id = null, but since the id parameter will be ignored in the characteristic case of idtype = P_ALL, it can allow PHP programmers to better express their intent and help readability.

For example, compare:

pcntl_waitid(P_ALL, null, $info);

with

pcntl_waitid(P_ALL, 0, $info);

or even

pcntl_waitid(P_ALL, 42, $info);

They all do the same thing, but IMHO the first version best communicates to the person reading the PHP code that we don't really care about the value of the second param (id).

In the C code, the id_is_null value is not used anywhere, as passing 0 to the system call will suffice, but I'm not sure if it's possible to allow a parameter to be null without using this Z_PARAM_LONG_OR_NULL macro.

Z_PARAM_ZVAL(user_siginfo)
Z_PARAM_LONG(options)
ZEND_PARSE_PARAMETERS_END();

errno = 0;
siginfo_t siginfo;

int status = waitid((idtype_t) idtype, (id_t) id, &siginfo, (int) options);

if (status == -1) {
PCNTL_G(last_error) = errno;
RETURN_FALSE;
}

pcntl_siginfo_to_zval(SIGCHLD, &siginfo, user_siginfo);

RETURN_TRUE;
}
#endif

/* {{{ Waits on or returns the status of a forked child as defined by the waitpid() system call */
PHP_FUNCTION(pcntl_wait)
{
Expand Down Expand Up @@ -1685,7 +1733,7 @@ PHP_FUNCTION(pcntl_setcpuaffinity)
zend_argument_value_error(2, "cpu id must be between 0 and " ZEND_ULONG_FMT " (" ZEND_LONG_FMT ")", maxcpus, cpu);
RETURN_THROWS();
}

if (!PCNTL_CPU_ISSET(cpu, mask)) {
PCNTL_CPU_SET(cpu, mask);
}
Expand Down
81 changes: 81 additions & 0 deletions ext/pcntl/pcntl.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,82 @@
*/
const WCONTINUED = UNKNOWN;
#endif
#if defined (HAVE_DECL_WEXITED) && HAVE_DECL_WEXITED == 1
/**
* @var int
* @cvalue LONG_CONST(WEXITED)
*/
const WEXITED = UNKNOWN;
#endif
#if defined (HAVE_DECL_WSTOPPED) && HAVE_DECL_WSTOPPED == 1
/**
* @var int
* @cvalue LONG_CONST(WSTOPPED)
*/
const WSTOPPED = UNKNOWN;
#endif
#if defined (HAVE_DECL_WNOWAIT) && HAVE_DECL_WNOWAIT== 1
/**
* @var int
* @cvalue LONG_CONST(WNOWAIT)
*/
const WNOWAIT = UNKNOWN;
#endif

#ifdef HAVE_WAITID
/* First argument to waitid */
#ifdef HAVE_POSIX_IDTYPES
/**
* @var int
* @cvalue LONG_CONST(P_ALL)
*/
const P_ALL = UNKNOWN;
/**
* @var int
* @cvalue LONG_CONST(P_PID)
*/
const P_PID = UNKNOWN;
/**
* @var int
* @cvalue LONG_CONST(P_PGID)
*/
const P_PGID = UNKNOWN;
#endif
/* Linux specific idtype */
#ifdef HAVE_LINUX_IDTYPES
/**
* @var int
* @cvalue LONG_CONST(P_PIDFD)
*/
const P_PIDFD = UNKNOWN;
#endif
/* NetBSD specific idtypes */
#ifdef HAVE_NETBSD_IDTYPES
/**
* @var int
* @cvalue LONG_CONST(P_UID)
*/
const P_UID = UNKNOWN;
/**
* @var int
* @cvalue LONG_CONST(P_GID)
*/
const P_GID = UNKNOWN;
/**
* @var int
* @cvalue LONG_CONST(P_SID)
*/
const P_SID = UNKNOWN;
#endif
/* FreeBSD specific idtype */
#ifdef HAVE_FREEBSD_IDTYPES
/**
* @var int
* @cvalue LONG_CONST(P_JAILID)
*/
const P_JAILID = UNKNOWN;
#endif
#endif

/* Signal Constants */

Expand Down Expand Up @@ -927,6 +1003,11 @@ function pcntl_fork(): int {}
*/
function pcntl_waitpid(int $process_id, &$status, int $flags = 0, &$resource_usage = []): int {}

#if defined (HAVE_WAITID) && defined (HAVE_POSIX_IDTYPES) && defined (HAVE_DECL_WEXITED) && HAVE_DECL_WEXITED == 1
/** @param array $info */
function pcntl_waitid(int $idtype = P_ALL, ?int $id = null, &$info = [], int $flags = WEXITED): bool {}
#endif

/**
* @param int $status
* @param array $resource_usage
Expand Down
50 changes: 49 additions & 1 deletion ext/pcntl/pcntl_arginfo.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ext/pcntl/php_pcntl.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#ifndef PHP_PCNTL_H
#define PHP_PCNTL_H

#if defined(WCONTINUED) && defined(WIFCONTINUED)
#if defined(HAVE_DECL_WCONTINUED) && HAVE_DECL_WCONTINUED == 1 && defined(HAVE_WIFCONTINUED) && HAVE_WIFCONTINUED == 1
#define HAVE_WCONTINUED 1
#endif

Expand Down
41 changes: 41 additions & 0 deletions ext/pcntl/tests/pcntl_waitid.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
--TEST--
pcntl_waitid()
--EXTENSIONS--
pcntl
posix
--SKIPIF--
<?php
if (!function_exists('pcntl_waitid')) die('skip pcntl_waitid unavailable');
?>
--FILE--
<?php
$pid = pcntl_fork();
if ($pid == -1) {
die("failed");
} else if ($pid) {
// invalid flags
var_dump(pcntl_waitid(P_PID, $pid, $siginfo, 0));
var_dump(pcntl_get_last_error() == PCNTL_EINVAL);

var_dump(pcntl_waitid(P_PID, $pid, $siginfo, WSTOPPED));
posix_kill($pid, SIGCONT);
var_dump(pcntl_waitid(P_PID, $pid, $siginfo, WCONTINUED));
posix_kill($pid, SIGUSR1);
var_dump(pcntl_waitid(P_PID, $pid, $siginfo, WEXITED));
var_dump($siginfo["status"]);
} else {
pcntl_signal(SIGUSR1, function ($_signo, $_siginfo) { exit(42); });
posix_kill(posix_getpid(), SIGSTOP);
pcntl_signal_dispatch();
sleep(42);
pcntl_signal_dispatch();
exit(6);
}
?>
--EXPECTF--
bool(false)
bool(true)
bool(true)
bool(true)
bool(true)
int(42)
Loading