Skip to content

Commit 23bb1c9

Browse files
frodeborlifrodeborli
authored
Fix stream_select() failing unpredictably with many or large descriptors (#14452)
Updated stream_select() so that its not limited by FD_SETSIZE anymore
1 parent 2544d0b commit 23bb1c9

12 files changed

Lines changed: 625 additions & 128 deletions

‎NEWS‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,15 @@ PHP NEWS
122122
lookup, and an unchecked CreateFileA() failure. (Ilia Alshanetsky)
123123
. Fix persistent stream context lifetime during shutdown (Levi Morrison)
124124
. Fixed bug GH-23780 (unserialize('') fails silently). (timwolla)
125+
. Fixed stream_select() aborting the whole call once any watched stream's
126+
descriptor exceeded FD_SETSIZE; it no longer imposes that limit, so watching
127+
many descriptors no longer requires rebuilding with --enable-fd-setsize.
128+
(Frode Børli)
129+
. Fixed stream_select() on Windows capping the number of watched sockets at
130+
FD_SETSIZE; the winsock fd_set now grows on demand. (Frode Børli)
131+
. Fixed stream_select() on Windows overflowing a fixed 64-entry stack array
132+
(crashing with a stack buffer overrun) when passed more than 64 pipe or file
133+
handles; it now fails gracefully instead. (Frode Børli)
125134
. Improved performance of array_splice() when inserting without removing
126135
elements. (mehmetcansahin)
127136
. Enforce max_filter_count: limit the number of filters that can be chained

‎UPGRADING‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,14 @@ PHP 8.7 UPGRADE NOTES
7373
5. Changed Functions
7474
========================================
7575

76+
- Standard:
77+
. stream_select() no longer fails once a watched descriptor exceeds
78+
FD_SETSIZE. The descriptor set now grows on demand on both POSIX (bitset)
79+
and Windows (winsock fd_set), so code watching many streams works without
80+
rebuilding PHP with --enable-fd-setsize. On Windows, selecting on more than
81+
64 pipe or file handles no longer crashes; it fails with a warning, as that
82+
path remains bounded by WaitForMultipleObjects().
83+
7684
========================================
7785
6. New Functions
7886
========================================

‎ext/standard/streamsfuncs.c‎

Lines changed: 44 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ PHP_FUNCTION(stream_clear_errors)
648648
}
649649

650650
/* {{{ stream_select related functions */
651-
static int stream_array_to_fd_set(const HashTable *stream_array, fd_set *fds, php_socket_t *max_fd)
651+
static int stream_array_to_fd_set(const HashTable *stream_array, php_growable_fd_set *fds, php_socket_t *max_fd)
652652
{
653653
zval *elem;
654654
php_stream *stream;
@@ -672,7 +672,7 @@ static int stream_array_to_fd_set(const HashTable *stream_array, fd_set *fds, ph
672672
* */
673673
if (SUCCESS == php_stream_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL, (void*)&this_fd, 1) && this_fd != -1) {
674674

675-
PHP_SAFE_FD_SET(this_fd, fds);
675+
php_growable_fd_set_add(fds, this_fd);
676676

677677
if (this_fd > *max_fd) {
678678
*max_fd = this_fd;
@@ -683,19 +683,14 @@ static int stream_array_to_fd_set(const HashTable *stream_array, fd_set *fds, ph
683683
return cnt ? 1 : 0;
684684
}
685685

686-
static int stream_array_from_fd_set(zval *stream_array, const fd_set *fds)
686+
static int stream_array_from_fd_set(zval *stream_array, const php_growable_fd_set *fds)
687687
{
688-
zval *elem, *dest_elem;
689-
HashTable *ht;
690-
php_stream *stream;
691-
int ret = 0;
692-
zend_string *key;
693-
zend_ulong num_ind;
694-
695688
ZEND_ASSERT(Z_TYPE_P(stream_array) == IS_ARRAY);
696-
ht = zend_new_array(zend_hash_num_elements(Z_ARRVAL_P(stream_array)));
689+
HashTable *ht = zend_new_array(zend_hash_num_elements(Z_ARRVAL_P(stream_array)));
690+
int ret = 0;
697691

698-
ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(stream_array), num_ind, key, elem) {
692+
ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(stream_array), zend_ulong num_ind, zend_string *key, zval *elem) {
693+
php_stream *stream;
699694
php_socket_t this_fd;
700695

701696
ZVAL_DEREF(elem);
@@ -709,7 +704,8 @@ static int stream_array_from_fd_set(zval *stream_array, const fd_set *fds)
709704
* is not displayed.
710705
*/
711706
if (SUCCESS == php_stream_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL, (void*)&this_fd, 1) && this_fd != SOCK_ERR) {
712-
if (PHP_SAFE_FD_ISSET(this_fd, fds)) {
707+
if (php_growable_fd_set_isset(fds, this_fd)) {
708+
zval *dest_elem;
713709
if (!key) {
714710
dest_elem = zend_hash_index_update(ht, num_ind, elem);
715711
} else {
@@ -780,15 +776,9 @@ static int stream_array_emulate_read_fd_set(zval *stream_array)
780776
PHP_FUNCTION(stream_select)
781777
{
782778
zval *r_array, *w_array, *e_array, *zcontext = NULL;
783-
struct timeval tv, *tv_p = NULL;
784-
fd_set rfds, wfds, efds;
785-
php_socket_t max_fd = 0;
786-
int retval, sets = 0;
787779
zend_long sec, usec = 0;
788780
bool secnull;
789781
bool usecnull = 1;
790-
int set_count, max_set_count = 0;
791-
php_stream_context *context = NULL;
792782

793783
ZEND_PARSE_PARAMETERS_START(4, 6)
794784
Z_PARAM_ARRAY_EX2(r_array, 1, 1, 0)
@@ -800,62 +790,52 @@ PHP_FUNCTION(stream_select)
800790
Z_PARAM_RESOURCE_OR_NULL(zcontext)
801791
ZEND_PARSE_PARAMETERS_END();
802792

803-
FD_ZERO(&rfds);
804-
FD_ZERO(&wfds);
805-
FD_ZERO(&efds);
806-
807793
php_stream_error_operation_begin();
808-
context = php_stream_context_from_zval(zcontext, 0);
794+
php_stream_context *context = php_stream_context_from_zval(zcontext, 0);
795+
796+
/* The sets grow as streams are added, past FD_SETSIZE */
797+
php_growable_fd_set rfds, wfds, efds;
798+
php_growable_fd_set_init(&rfds, FD_SETSIZE);
799+
php_growable_fd_set_init(&wfds, FD_SETSIZE);
800+
php_growable_fd_set_init(&efds, FD_SETSIZE);
809801

802+
php_socket_t max_fd = 0;
803+
int sets = 0;
810804
if (r_array != NULL) {
811-
set_count = stream_array_to_fd_set(Z_ARR_P(r_array), &rfds, &max_fd);
812-
if (set_count > max_set_count)
813-
max_set_count = set_count;
814-
sets += set_count;
805+
sets += stream_array_to_fd_set(Z_ARR_P(r_array), &rfds, &max_fd);
815806
}
816-
817807
if (w_array != NULL) {
818-
set_count = stream_array_to_fd_set(Z_ARR_P(w_array), &wfds, &max_fd);
819-
if (set_count > max_set_count)
820-
max_set_count = set_count;
821-
sets += set_count;
808+
sets += stream_array_to_fd_set(Z_ARR_P(w_array), &wfds, &max_fd);
822809
}
823-
824810
if (e_array != NULL) {
825-
set_count = stream_array_to_fd_set(Z_ARR_P(e_array), &efds, &max_fd);
826-
if (set_count > max_set_count)
827-
max_set_count = set_count;
828-
sets += set_count;
811+
sets += stream_array_to_fd_set(Z_ARR_P(e_array), &efds, &max_fd);
829812
}
830813

831814
if (!sets) {
832815
php_stream_error_operation_end(context);
833816
zend_value_error("No stream arrays were passed");
834-
RETURN_THROWS();
835-
}
836-
837-
if (!PHP_SAFE_MAX_FD(max_fd, max_set_count)) {
838-
RETURN_FALSE;
817+
goto cleanup;
839818
}
840819

841820
if (secnull && !usecnull) {
842821
if (usec != 0) {
843822
php_stream_error_operation_end(context);
844823
zend_argument_value_error(5, "must be null when argument #4 ($seconds) is null");
845-
RETURN_THROWS();
824+
goto cleanup;
846825
}
847826
}
848827

849828
/* If seconds is not set to null, build the timeval, else we wait indefinitely */
829+
struct timeval tv, *tv_p = NULL;
850830
if (!secnull) {
851831
if (sec < 0) {
852832
php_stream_error_operation_end(context);
853833
zend_argument_value_error(4, "must be greater than or equal to 0");
854-
RETURN_THROWS();
834+
goto cleanup;
855835
} else if (usec < 0) {
856836
php_stream_error_operation_end(context);
857837
zend_argument_value_error(5, "must be greater than or equal to 0");
858-
RETURN_THROWS();
838+
goto cleanup;
859839
}
860840

861841
/* Windows, Solaris and BSD do not like microsecond values which are >= 1 sec */
@@ -867,6 +847,7 @@ PHP_FUNCTION(stream_select)
867847
/* slight hack to support buffered data; if there is data sitting in the
868848
* read buffer of any of the streams in the read array, let's pretend
869849
* that we selected, but return only the readable sockets */
850+
int retval;
870851
if (r_array != NULL) {
871852
retval = stream_array_emulate_read_fd_set(r_array);
872853
if (retval > 0) {
@@ -879,24 +860,37 @@ PHP_FUNCTION(stream_select)
879860
zval_ptr_dtor(e_array);
880861
ZVAL_EMPTY_ARRAY(e_array);
881862
}
882-
RETURN_LONG(retval);
863+
RETVAL_LONG(retval);
864+
goto cleanup;
883865
}
884866
}
885867

886-
retval = php_select(max_fd+1, &rfds, &wfds, &efds, tv_p);
868+
#ifndef PHP_WIN32
869+
/* select() reads max_fd + 1 bits of each set */
870+
php_growable_fd_set_reserve(&rfds, max_fd + 1);
871+
php_growable_fd_set_reserve(&wfds, max_fd + 1);
872+
php_growable_fd_set_reserve(&efds, max_fd + 1);
873+
#endif
874+
retval = php_select(max_fd + 1, rfds.set, wfds.set, efds.set, tv_p);
887875
php_stream_error_operation_end(context);
888876

889877
if (retval == -1) {
890878
php_error_docref(NULL, E_WARNING, "Unable to select [%d]: %s (max_fd=" PHP_SOCKET_FMT ")",
891879
errno, strerror(errno), max_fd);
892-
RETURN_FALSE;
880+
RETVAL_FALSE;
881+
goto cleanup;
893882
}
894883

895884
if (r_array != NULL) stream_array_from_fd_set(r_array, &rfds);
896885
if (w_array != NULL) stream_array_from_fd_set(w_array, &wfds);
897886
if (e_array != NULL) stream_array_from_fd_set(e_array, &efds);
898887

899-
RETURN_LONG(retval);
888+
RETVAL_LONG(retval);
889+
890+
cleanup:
891+
php_growable_fd_set_destroy(&rfds);
892+
php_growable_fd_set_destroy(&wfds);
893+
php_growable_fd_set_destroy(&efds);
900894
}
901895
/* }}} */
902896

‎ext/standard/tests/streams/gh9590-001.phpt‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
--TEST--
2-
Bug GH-9590 001 (stream_select does not abort upon exception or empty valid fd set)
2+
Bug GH-9590 001 (stream_select works correctly, without warning, past the traditional
3+
FD_SETSIZE descriptor limit on platforms where that limit has been lifted)
34
--EXTENSIONS--
45
posix
56
--SKIPIF--
67
<?php
7-
if (!function_exists('posix_setrlimit') || !posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, -1)) {
8+
// Keep the hard limit: raising it to unlimited fails even for root on Linux (fs.nr_open)
9+
$hard = function_exists('posix_getrlimit') ? posix_getrlimit(POSIX_RLIMIT_NOFILE)[1] : -1;
10+
if (!function_exists('posix_setrlimit') || !posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, is_int($hard) ? $hard : -1)) {
811
die('skip Failed to set POSIX_RLIMIT_NOFILE');
912
}
1013
if (PHP_OS_FAMILY === 'Solaris' && PHP_INT_SIZE === 8)
@@ -13,27 +16,26 @@ if (PHP_OS_FAMILY === 'Solaris' && PHP_INT_SIZE === 8)
1316
--FILE--
1417
<?php
1518

16-
posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, -1);
19+
$hard = posix_getrlimit(POSIX_RLIMIT_NOFILE)[1];
20+
posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, is_int($hard) ? $hard : -1);
1721

1822
$fds = [];
1923
for ($i = 0; $i < 1023; $i++) {
2024
$fds[] = @fopen(__DIR__ . "/GH-9590-001-tmpfile.$i", 'w');
2125
}
2226

2327
list($a, $b) = stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, STREAM_IPPROTO_IP);
28+
fwrite($b, "x");
2429

2530
$r = [$a];
2631
$w = $e = [];
27-
var_dump(stream_select($r, $w, $e, PHP_INT_MAX));
32+
var_dump(stream_select($r, $w, $e, 30));
33+
var_dump(fread($a, 1));
2834

2935
?>
30-
--EXPECTF--
31-
Warning: stream_select(): You MUST recompile PHP with a larger value of FD_SETSIZE.
32-
It is set to 1024, but you have descriptors numbered at least as high as %d.
33-
--enable-fd-setsize=%d is recommended, but you may want to set it
34-
to equal the maximum number of open files supported by your system,
35-
in order to avoid seeing this error again at a later date. in %s on line %d
36-
bool(false)
36+
--EXPECT--
37+
int(1)
38+
string(1) "x"
3739
--CLEAN--
3840
<?php
3941
for ($i = 0; $i < 1023; $i++) {

‎ext/standard/tests/streams/gh9590-002.phpt‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
--TEST--
2-
Bug GH-9590 002 (stream_select does not abort upon exception or empty valid fd set)
2+
Bug GH-9590 002 (a custom error handler that turns warnings into exceptions must not be
3+
spuriously triggered by stream_select() past the traditional FD_SETSIZE limit, on
4+
platforms where that limit has been lifted)
35
--EXTENSIONS--
46
posix
57
--SKIPIF--
68
<?php
7-
if (!function_exists('posix_setrlimit') || !posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, -1)) {
9+
// Keep the hard limit: raising it to unlimited fails even for root on Linux (fs.nr_open)
10+
$hard = function_exists('posix_getrlimit') ? posix_getrlimit(POSIX_RLIMIT_NOFILE)[1] : -1;
11+
if (!function_exists('posix_setrlimit') || !posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, is_int($hard) ? $hard : -1)) {
812
die('skip Failed to set POSIX_RLIMIT_NOFILE');
913
}
1014
if (PHP_OS_FAMILY === 'Solaris' && PHP_INT_SIZE === 8)
@@ -13,29 +17,28 @@ if (PHP_OS_FAMILY === 'Solaris' && PHP_INT_SIZE === 8)
1317
--FILE--
1418
<?php
1519

16-
posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, -1);
20+
$hard = posix_getrlimit(POSIX_RLIMIT_NOFILE)[1];
21+
posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, is_int($hard) ? $hard : -1);
1722

1823
$fds = [];
1924
for ($i = 0; $i < 1023; $i++) {
2025
$fds[] = @fopen(__DIR__ . "/GH-9590-002-tmpfile.$i", 'w');
2126
}
2227

2328
list($a, $b) = stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, STREAM_IPPROTO_IP);
29+
fwrite($b, "x");
2430

2531
set_error_handler(function($errno, $errstr) { throw new \Exception($errstr); });
2632

2733
$r = [$a];
2834
$w = $e = [];
29-
var_dump(stream_select($r, $w, $e, PHP_INT_MAX));
35+
var_dump(stream_select($r, $w, $e, 30));
36+
var_dump(fread($a, 1));
3037

3138
?>
32-
--EXPECTF--
33-
Fatal error: Uncaught Exception: stream_select(): You MUST recompile PHP with a larger value of FD_SETSIZE.
34-
It is set to 1024, but you have descriptors numbered at least as high as %d.
35-
--enable-fd-setsize=%d is recommended, but you may want to set it
36-
to equal the maximum number of open files supported by your system,
37-
in order to avoid seeing this error again at a later date. in %s:%d
38-
Stack trace:%a
39+
--EXPECT--
40+
int(1)
41+
string(1) "x"
3942
--CLEAN--
4043
<?php
4144
for ($i = 0; $i < 1023; $i++) {

0 commit comments

Comments
 (0)