Fix stream_select() failing unpredictably (and crashing on Windows) with many descriptors

NEWS

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

UPGRADING

@@ -45,6 +45,14 @@ PHP 8.7 UPGRADE NOTES
 5. Changed Functions
 ========================================
 
+- Standard:
+  . stream_select() no longer fails once a watched descriptor exceeds
+    FD_SETSIZE. The descriptor set now grows on demand on both POSIX (bitset)
+    and Windows (winsock fd_set), so code watching many streams works without
+    rebuilding PHP with --enable-fd-setsize. On Windows, selecting on more than
+    64 pipe or file handles no longer crashes; it fails with a warning, as that
+    path remains bounded by WaitForMultipleObjects().
+
 ========================================
 6. New Functions
 ========================================

ext/standard/streamsfuncs.c

@@ -648,7 +648,7 @@ PHP_FUNCTION(stream_clear_errors)
 }
 
 /* {{{ stream_select related functions */
-static int stream_array_to_fd_set(const HashTable *stream_array, fd_set *fds, php_socket_t *max_fd)
+static int stream_array_to_fd_set(const HashTable *stream_array, php_growable_fd_set *fds, php_socket_t *max_fd)
 {
 	zval *elem;
 	php_stream *stream;
@@ -672,7 +672,7 @@ static int stream_array_to_fd_set(const HashTable *stream_array, fd_set *fds, ph
 		 * */
 		if (SUCCESS == php_stream_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL, (void*)&this_fd, 1) && this_fd != -1) {
 
-			PHP_SAFE_FD_SET(this_fd, fds);
+			php_growable_fd_set_add(fds, this_fd);
 
 			if (this_fd > *max_fd) {
 				*max_fd = this_fd;
@@ -683,19 +683,14 @@ static int stream_array_to_fd_set(const HashTable *stream_array, fd_set *fds, ph
 	return cnt ? 1 : 0;
 }
 
-static int stream_array_from_fd_set(zval *stream_array, const fd_set *fds)
+static int stream_array_from_fd_set(zval *stream_array, const php_growable_fd_set *fds)
 {
-	zval *elem, *dest_elem;
-	HashTable *ht;
-	php_stream *stream;
-	int ret = 0;
-	zend_string *key;
-	zend_ulong num_ind;
-
 	ZEND_ASSERT(Z_TYPE_P(stream_array) == IS_ARRAY);
-	ht = zend_new_array(zend_hash_num_elements(Z_ARRVAL_P(stream_array)));
+	HashTable *ht = zend_new_array(zend_hash_num_elements(Z_ARRVAL_P(stream_array)));
+	int ret = 0;
 
-	ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(stream_array), num_ind, key, elem) {
+	ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(stream_array), zend_ulong num_ind, zend_string *key, zval *elem) {
+		php_stream *stream;
 		php_socket_t this_fd;
 
 		ZVAL_DEREF(elem);
@@ -709,7 +704,8 @@ static int stream_array_from_fd_set(zval *stream_array, const fd_set *fds)
 		 * is not displayed.
 		 */
 		if (SUCCESS == php_stream_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL, (void*)&this_fd, 1) && this_fd != SOCK_ERR) {
-			if (PHP_SAFE_FD_ISSET(this_fd, fds)) {
+			if (php_growable_fd_set_isset(fds, this_fd)) {
+				zval *dest_elem;
 				if (!key) {
 					dest_elem = zend_hash_index_update(ht, num_ind, elem);
 				} else {
@@ -780,15 +776,9 @@ static int stream_array_emulate_read_fd_set(zval *stream_array)
 PHP_FUNCTION(stream_select)
 {
 	zval *r_array, *w_array, *e_array, *zcontext = NULL;
-	struct timeval tv, *tv_p = NULL;
-	fd_set rfds, wfds, efds;
-	php_socket_t max_fd = 0;
-	int retval, sets = 0;
 	zend_long sec, usec = 0;
 	bool secnull;
 	bool usecnull = 1;
-	int set_count, max_set_count = 0;
-	php_stream_context *context = NULL;
 
 	ZEND_PARSE_PARAMETERS_START(4, 6)
 		Z_PARAM_ARRAY_EX2(r_array, 1, 1, 0)
@@ -800,62 +790,52 @@ PHP_FUNCTION(stream_select)
 		Z_PARAM_RESOURCE_OR_NULL(zcontext)
 	ZEND_PARSE_PARAMETERS_END();
 
-	FD_ZERO(&rfds);
-	FD_ZERO(&wfds);
-	FD_ZERO(&efds);
-
 	php_stream_error_operation_begin();
-	context = php_stream_context_from_zval(zcontext, 0);
+	php_stream_context *context = php_stream_context_from_zval(zcontext, 0);
+
+	/* The sets grow as streams are added, past FD_SETSIZE */
+	php_growable_fd_set rfds, wfds, efds;
+	php_growable_fd_set_init(&rfds, FD_SETSIZE);
+	php_growable_fd_set_init(&wfds, FD_SETSIZE);
+	php_growable_fd_set_init(&efds, FD_SETSIZE);
 
+	php_socket_t max_fd = 0;
+	int sets = 0;
 	if (r_array != NULL) {
-		set_count = stream_array_to_fd_set(Z_ARR_P(r_array), &rfds, &max_fd);
-		if (set_count > max_set_count)
-			max_set_count = set_count;
-		sets += set_count;
+		sets += stream_array_to_fd_set(Z_ARR_P(r_array), &rfds, &max_fd);
 	}
-
 	if (w_array != NULL) {
-		set_count = stream_array_to_fd_set(Z_ARR_P(w_array), &wfds, &max_fd);
-		if (set_count > max_set_count)
-			max_set_count = set_count;
-		sets += set_count;
+		sets += stream_array_to_fd_set(Z_ARR_P(w_array), &wfds, &max_fd);
 	}
-
 	if (e_array != NULL) {
-		set_count = stream_array_to_fd_set(Z_ARR_P(e_array), &efds, &max_fd);
-		if (set_count > max_set_count)
-			max_set_count = set_count;
-		sets += set_count;
+		sets += stream_array_to_fd_set(Z_ARR_P(e_array), &efds, &max_fd);
 	}
 
 	if (!sets) {
 		php_stream_error_operation_end(context);
 		zend_value_error("No stream arrays were passed");
-		RETURN_THROWS();
-	}
-
-	if (!PHP_SAFE_MAX_FD(max_fd, max_set_count)) {
-		RETURN_FALSE;
+		goto cleanup;
 	}
 
 	if (secnull && !usecnull) {
 		if (usec != 0) {
 			php_stream_error_operation_end(context);
 			zend_argument_value_error(5, "must be null when argument #4 ($seconds) is null");
-			RETURN_THROWS();
+			goto cleanup;
 		}
 	}
 
 	/* If seconds is not set to null, build the timeval, else we wait indefinitely */
+	struct timeval tv, *tv_p = NULL;
 	if (!secnull) {
 		if (sec < 0) {
 			php_stream_error_operation_end(context);
 			zend_argument_value_error(4, "must be greater than or equal to 0");
-			RETURN_THROWS();
+			goto cleanup;
 		} else if (usec < 0) {
 			php_stream_error_operation_end(context);
 			zend_argument_value_error(5, "must be greater than or equal to 0");
-			RETURN_THROWS();
+			goto cleanup;
 		}
 
 		/* Windows, Solaris and BSD do not like microsecond values which are >= 1 sec */
@@ -867,6 +847,7 @@ PHP_FUNCTION(stream_select)
 	/* slight hack to support buffered data; if there is data sitting in the
 	 * read buffer of any of the streams in the read array, let's pretend
 	 * that we selected, but return only the readable sockets */
+	int retval;
 	if (r_array != NULL) {
 		retval = stream_array_emulate_read_fd_set(r_array);
 		if (retval > 0) {
@@ -879,24 +860,37 @@ PHP_FUNCTION(stream_select)
 				zval_ptr_dtor(e_array);
 				ZVAL_EMPTY_ARRAY(e_array);
 			}
-			RETURN_LONG(retval);
+			RETVAL_LONG(retval);
+			goto cleanup;
 		}
 	}
 
-	retval = php_select(max_fd+1, &rfds, &wfds, &efds, tv_p);
+#ifndef PHP_WIN32
+	/* select() reads max_fd + 1 bits of each set */
+	php_growable_fd_set_reserve(&rfds, max_fd + 1);
+	php_growable_fd_set_reserve(&wfds, max_fd + 1);
+	php_growable_fd_set_reserve(&efds, max_fd + 1);
+#endif
+	retval = php_select(max_fd + 1, rfds.set, wfds.set, efds.set, tv_p);
 	php_stream_error_operation_end(context);
 
 	if (retval == -1) {
 		php_error_docref(NULL, E_WARNING, "Unable to select [%d]: %s (max_fd=" PHP_SOCKET_FMT ")",
 				errno, strerror(errno), max_fd);
-		RETURN_FALSE;
+		RETVAL_FALSE;
+		goto cleanup;
 	}
 
 	if (r_array != NULL) stream_array_from_fd_set(r_array, &rfds);
 	if (w_array != NULL) stream_array_from_fd_set(w_array, &wfds);
 	if (e_array != NULL) stream_array_from_fd_set(e_array, &efds);
 
-	RETURN_LONG(retval);
+	RETVAL_LONG(retval);
+
+cleanup:
+	php_growable_fd_set_destroy(&rfds);
+	php_growable_fd_set_destroy(&wfds);
+	php_growable_fd_set_destroy(&efds);
 }
 /* }}} */
 

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

@@ -1,10 +1,13 @@
 --TEST--
-Bug GH-9590 001 (stream_select does not abort upon exception or empty valid fd set)
+Bug GH-9590 001 (stream_select works correctly, without warning, past the traditional
+FD_SETSIZE descriptor limit on platforms where that limit has been lifted)
 --EXTENSIONS--
 posix
 --SKIPIF--
 <?php
-if (!function_exists('posix_setrlimit') || !posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, -1)) {
+// Keep the hard limit: raising it to unlimited fails even for root on Linux (fs.nr_open)
+$hard = function_exists('posix_getrlimit') ? posix_getrlimit(POSIX_RLIMIT_NOFILE)[1] : -1;
+if (!function_exists('posix_setrlimit') || !posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, is_int($hard) ? $hard : -1)) {
     die('skip Failed to set POSIX_RLIMIT_NOFILE');
 }
 if (PHP_OS_FAMILY === 'Solaris' && PHP_INT_SIZE === 8)
@@ -13,27 +16,26 @@ if (PHP_OS_FAMILY === 'Solaris' && PHP_INT_SIZE === 8)
 --FILE--
 <?php
 
-posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, -1);
+$hard = posix_getrlimit(POSIX_RLIMIT_NOFILE)[1];
+posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, is_int($hard) ? $hard : -1);
 
 $fds = [];
 for ($i = 0; $i < 1023; $i++) {
     $fds[] = @fopen(__DIR__ . "/GH-9590-001-tmpfile.$i", 'w');
 }
 
 list($a, $b) = stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, STREAM_IPPROTO_IP);
+fwrite($b, "x");
 
 $r = [$a];
 $w = $e = [];
-var_dump(stream_select($r, $w, $e, PHP_INT_MAX));
+var_dump(stream_select($r, $w, $e, 30));
+var_dump(fread($a, 1));
 
 ?>
---EXPECTF--
-Warning: stream_select(): You MUST recompile PHP with a larger value of FD_SETSIZE.
-It is set to 1024, but you have descriptors numbered at least as high as %d.
- --enable-fd-setsize=%d is recommended, but you may want to set it
-to equal the maximum number of open files supported by your system,
-in order to avoid seeing this error again at a later date. in %s on line %d
-bool(false)
+--EXPECT--
+int(1)
+string(1) "x"
 --CLEAN--
 <?php
 for ($i = 0; $i < 1023; $i++) {

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

@@ -1,10 +1,14 @@
 --TEST--
-Bug GH-9590 002 (stream_select does not abort upon exception or empty valid fd set)
+Bug GH-9590 002 (a custom error handler that turns warnings into exceptions must not be
+spuriously triggered by stream_select() past the traditional FD_SETSIZE limit, on
+platforms where that limit has been lifted)
 --EXTENSIONS--
 posix
 --SKIPIF--
 <?php
-if (!function_exists('posix_setrlimit') || !posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, -1)) {
+// Keep the hard limit: raising it to unlimited fails even for root on Linux (fs.nr_open)
+$hard = function_exists('posix_getrlimit') ? posix_getrlimit(POSIX_RLIMIT_NOFILE)[1] : -1;
+if (!function_exists('posix_setrlimit') || !posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, is_int($hard) ? $hard : -1)) {
     die('skip Failed to set POSIX_RLIMIT_NOFILE');
 }
 if (PHP_OS_FAMILY === 'Solaris' && PHP_INT_SIZE === 8)
@@ -13,29 +17,28 @@ if (PHP_OS_FAMILY === 'Solaris' && PHP_INT_SIZE === 8)
 --FILE--
 <?php
 
-posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, -1);
+$hard = posix_getrlimit(POSIX_RLIMIT_NOFILE)[1];
+posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, is_int($hard) ? $hard : -1);
 
 $fds = [];
 for ($i = 0; $i < 1023; $i++) {
     $fds[] = @fopen(__DIR__ . "/GH-9590-002-tmpfile.$i", 'w');
 }
 
 list($a, $b) = stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, STREAM_IPPROTO_IP);
+fwrite($b, "x");
 
 set_error_handler(function($errno, $errstr) { throw new \Exception($errstr); });
 
 $r = [$a];
 $w = $e = [];
-var_dump(stream_select($r, $w, $e, PHP_INT_MAX));
+var_dump(stream_select($r, $w, $e, 30));
+var_dump(fread($a, 1));
 
 ?>
---EXPECTF--
-Fatal error: Uncaught Exception: stream_select(): You MUST recompile PHP with a larger value of FD_SETSIZE.
-It is set to 1024, but you have descriptors numbered at least as high as %d.
- --enable-fd-setsize=%d is recommended, but you may want to set it
-to equal the maximum number of open files supported by your system,
-in order to avoid seeing this error again at a later date. in %s:%d
-Stack trace:%a
+--EXPECT--
+int(1)
+string(1) "x"
 --CLEAN--
 <?php
 for ($i = 0; $i < 1023; $i++) {