-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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
feof() behavior change for UNIX based socket resources in PHP 8.2. #10406
Comments
I tend to agree. (And thank you for looking into this!) |
@MaxKellermann could you have a look at this? |
Calling Prior to my change, the
(PS: the After my change, there's no
The This is indeed not an intentional change, but it's undefined behavior, so it's rather: the old return value was not intentional; it should always have returned an error. If you want to restore the old behavior, you'd either need to revert my patch, or add special checks to ignore the error condition. I wouldn't do either; I'd just leave it as it is, and document that this call is (and always has been) illegal. PS2: this is how it looks like (with PHP 7.4, i.e. prior to my change) if there was an incoming connection:
As you see, same error as with PHP 8.2! |
Also see
The socket in this issue was not opened by |
But why doesn't |
I guess the I guess I could easily add such a (userspace) flag and add a warning - do you want that? |
I suppose it might be possible to check the |
That would technically be possible, but appears rather fragile to me. What if new stream types get added? (By an extension?) Correct behavior should be ensured by good API design. |
Regarding good API design: I'd like to question the idea that a listener socket is exposed as "stream" in PHP .... a listener socket is all but a stream. No data is ever transferred over it. Streams and listeners have nothing in common. Even if they are both file descriptors at the system call level, exposing them as the same type of object in a high-level language never made sense. This mixup is what causes the confusion about using I wonder if it would be possible to expose listeners as a different built-in class? |
Adding a new member would likely constitute an ABI break, in which case we cannot fix this for PHP 8.2 (master would be fine). Using a fragile workaround as a stop gap measure might be sensible.
That would constitute a serious BC break, so cannot be done prior to PHP 9. Our long term goal is to get rid of all stream resources, but accomplishing that is hard (if not impossible) for practical reasons. |
I checked - no, it doesn't work.
Can't see whether this is a listener socket or a connection socket. |
Description
I have been using
feof()
as a way to determine if a socket resource is actively open. In hindsight, I should've usedis_resource
for this it seems, as looking at notes for that it will return false if the resource is closed. Regardless, in PHP 8.2 the behavior offeof()
changed for UNIX sockets. Unlike TCP based resources, it callingfeof
on it now returnstrue
even while the resource connection is still open. But I don't see anything in the changelog / release notes describing any change in behavior to this method. If this change was intentional, it seems like it should be documented somewhere? However, since the behavior with TCP isn't officially documented, maybe it's an unfortunate wash 🤷The code where I originally discovered this was while updating a personal project to verify PHP 8.2 compatibility: FreeDSx/LDAP#56, as that CI run shows that PHP 8.2 is the only version where the unix socket behavior is shown (it tests PHP 7.1 - 8.2 testing). I've simplified the behavior in the below script.
Given the following script:
Resulted in this output (PHP 8.2):
But I expected this output instead (PHP <= 8.1):
The behavior for checking a TCP based socket resource did not change however. It seems to be specific to UNIX based sockets for some reason.
PHP Version
PHP 8.2
Operating System
Ubuntu 22.04.1
The text was updated successfully, but these errors were encountered: