-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Return immediately when FD_SETSIZE is exceeded #9602
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just few NITS but otherwise all looks reasonable to me.
posix_setrlimit(POSIX_RLIMIT_NOFILE, 2048, -1); | ||
|
||
$fds = []; | ||
for ($i = 0; $i < 1023; $i++) { | ||
$fds[] = @fopen(__DIR__ . "/GH-9590-tmpfile.$i", 'w'); | ||
} | ||
|
||
require_once('connect.inc'); | ||
|
||
function get_connection() { | ||
global $host, $user, $passwd, $db, $port, $socket; | ||
|
||
if (!$link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket)) | ||
printf("[001] [%d] %s\n", mysqli_connect_errno(), mysqli_connect_error()); | ||
return $link; | ||
} | ||
|
||
|
||
$mysqli1 = get_connection(); | ||
$mysqli2 = get_connection(); | ||
|
||
var_dump(mysqli_query($mysqli1, "SELECT SLEEP(0.10)", MYSQLI_ASYNC | MYSQLI_USE_RESULT)); | ||
var_dump(mysqli_query($mysqli2, "SELECT SLEEP(0.20)", MYSQLI_ASYNC | MYSQLI_USE_RESULT)); | ||
|
||
$links = $errors = $reject = array($mysqli1, $mysqli2); | ||
var_dump(mysqli_poll($links, $errors, $reject, 0, 50000)); | ||
|
||
mysqli_close($mysqli1); | ||
mysqli_close($mysqli2); | ||
|
||
print "done!"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: extra indent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many tests in this directory are like that, I tried to keep the existing style
I guess just always returning is the easiest thing to do :-) I'm not sure whether it's better to generally allow graceful handling or just return, but I'm fine with this too. |
Co-authored-by: Jakub Zelenka <bukka@php.net>
Co-authored-by: Jakub Zelenka <bukka@php.net>
@bwoebi I'm open to suggestions. Do you have a solution in mind to handle this gracefully ? Currently this change causes select() to return false when FD_SETSIZE is exceeded instead of hanging forever. |
The other option is to just return when an exception is thrown or when the fd set is empty. But I'm not sure whether that's better. |
I have a preference for just returning when an exception is thrown, or for always returning when FD_SETSIZE is exceeded, over returning only when the fd set is empty. In the latter, the user may be under the impression that select() actually works despite the warning, because select() will monitor some FDs, but some others will not actually be monitored. Returning only when an exception is thrown has the advantage of making the least behavioral changes while still fixing the original bug. Returning when FD_SETSIZE also prevents hanging forever (in the case where the set is empty), and makes the problem more visible (if the warning is not spotted already). |
Fine by me :-) |
* PHP-8.0: [ci skip] NEWS Return immediately when FD_SETSIZE is exceeded (php#9602)
* PHP-8.1: [ci skip] NEWS [ci skip] NEWS Return immediately when FD_SETSIZE is exceeded (#9602)
* PHP-8.2: [ci skip] NEWS [ci skip] NEWS [ci skip] NEWS Return immediately when FD_SETSIZE is exceeded (#9602)
Fixes #9590