Bug #78038 [Csd]: Socket_select fails when resource array contains references

Edit report at https://bugs.php.net/bug.php?id=78038&edit=1 ID: 78038 Updated by: nikic@php.net Reported by: ogdg at protonmail dot com Summary: Socket_select fails when resource array contains references Status: Closed Type: Bug Package: Sockets related Operating System: Fedora 30 PHP Version: 7.3.5 -Assigned To: +Assigned To: nikic Block user comment: N Private report: N New Comment: I've fixed the issue with the reference handling, and think it's pretty likely that this is what's causing the problem for you. There's likely some singleton references lying around in your socket array that won't even show up in var_dump (e.g. if you do a by-ref iteration over the sockets or similar). If the issue persists after this fix, let's take another look. Previous Comments: ------------------------------------------------------------------------ [2019-05-23 09:13:40] nikic@php.net Automatic comment on behalf of nikita.ppv@gmail.com Revision: http://git.php.net/?p=php-src.git;a=commit;h=9a74b23297167ca7956fd21428c8b987e7d99aa5 Log: Fixed bug #78038 socket_select with references ------------------------------------------------------------------------ [2019-05-23 07:58:49] nikic@php.net > So in this case, 6a fails with the warnings listed in the original report, while 6b works fine. > In the actual application, the socket array is passed to a custom function by reference...but I did > try passing it directly and the result was the same in 6a. From looking at the code, I suspect that the problem does have something to do with references being introduced somewhere. At least we're missing a DEREF in here: https://github.com/php/php-src/blob/2e4686b5667e9ccbfbfab10452c42f5b818fcfdb/ext/sockets/sockets.c#L906 ------------------------------------------------------------------------ [2019-05-23 07:33:28] ogdg at protonmail dot com @nikic The latest version I was using that did not have this issue was 7.2, and it never happened in any of the version 5 series. I am using the version that is included with Fedora 30 standard repos without any modifications or custom extensions. Just the default install. @daverandom I made a short script to simulate what my application does but the error does not occur. The actual application is using AF_UNIX type sockets for IPC. I do not have the free time to reproduce a full-blown script of this sort right now, but the bug is still present. The application where this issue happens has the basic structure like this: 1) Main process creates AF_UNIX socket 2) Sub process launched, create their own AF_UNIX socket and connect to the main process socket 3) Main process does a socket_accept for new IPC connections 4) Main process uses a this to get PID of child process: $pid = socket_get_option($conn, SOL_SOCKET, 17); $pid = (int)$pid; 5) The main process keeps a copy of the resource in an array indexed by PIDs. $socketArray['ipc'][$pid]; 6a) Problem happens when I take that array and do this: $e = $w = NULL; $r = $socketArray['ipc']; socket_select($r, $w, $e, 0); 6b) To fix this, I had to manually loop through the resource array: foreach($socketArray['ipc'] as $v) $r[] = $v; ... So in this case, 6a fails with the warnings listed in the original report, while 6b works fine. In the actual application, the socket array is passed to a custom function by reference...but I did try passing it directly and the result was the same in 6a. Both print_r and var_dump show that the array is being correctly assigned in 6a, and that $r does indeed contain resources, which is why the fix in 6b works. I can't really tell what the problem is, but it's definitely there and seems to appear in very specific conditions. My first guess was that the $pid wasn't being cast to INT and was being treated as a string, so I cast it to INT and that didn't help. The other thing I thought could be a problem is the large numeric values for the $pid, meaning that for some reason socket_select chokes when the index values of any connection array are above a certain number, but I couldn't reproduce this in a standalone script just using large index values for socket_select...also, the standalone script is not using pcntl_fork as the actual application does. ------------------------------------------------------------------------ [2019-05-22 07:44:17] nikic@php.net One recent change comes to mind here: https://github.com/php/php-src/commit/bdac9ef10d90997f9576564dde693cbef6594142#diff-9df410db21c230cd2ec04a4c95ccd20f However, it is in stream_select(), not socket_select(). When you say "previous versions" do you mean 7.3.4 or some older version? ------------------------------------------------------------------------ [2019-05-20 10:00:10] daverandom@php.net Thank you for this bug report. To properly diagnose the problem, we need a short but complete example script to be able to reproduce this bug ourselves. A proper reproducing script starts with <?php and ends with ?>, is max. 10-20 lines long and does not require any external resources such as databases, etc. If the script requires a database to demonstrate the issue, please make sure it creates all necessary tables, stored procedures etc. Please avoid embedding huge scripts into the report. I cannot reproduce this in 7.3.5 or any other version. Additionally, none of the relevant code in php-src has been changed for several years in 7.3[1] or master[2]. Please recheck your code, var_dump($r) and verify that it contains at least one valid socket resource, and if so please provide a complete reproducing script. There is a lot of code in the wild which utilises a pattern roughly like below - it's likely that if it were broken then a lot of people would be shouting about it :-) $socket = socket_create(/* stuff */); // ...set up the socket $sockets[(int)$socket] = $socket; // Add socket to array indexed by resource ID $w = $e = NULL; $r = $sockets; $check = socket_select($r, $w, $e, 0); [1] https://github.com/php/php-src/blame/PHP-7.3/ext/sockets/sockets.c#L915-L977 [2] https://github.com/php/php-src/blame/master/ext/sockets/sockets.c#L921-L983 ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=78038 -- Edit this bug report at https://bugs.php.net/bug.php?id=78038&edit=1