Skip to content
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

ftp functions can abort with EINTR #16800

Open
bwoebi opened this issue Nov 14, 2024 · 5 comments
Open

ftp functions can abort with EINTR #16800

bwoebi opened this issue Nov 14, 2024 · 5 comments

Comments

@bwoebi
Copy link
Member

bwoebi commented Nov 14, 2024

Description

The underlying cause is poll(2) syscalls in the ext/ftp code (poll is one of those syscalls which cannot be resumed via SA_RESTART).
Short of guarding every single ftp operation, there's no way to cleanly resume ftp processing.
Especially given that the ext/ftp operations are sometimes big operations - like, I don't really have certainty on whether a transfer properly happened or not, I just know it was aborted.

my_send() and my_recv() in the ftp code both call php_pollfd_for_ms() and immediately abort on any non-zero return instead of retrying on EINTR.
The ext/ftp operations are high-level enough that they should handle EINTR themselves and resume accordingly.

PHP Version

PHP 8.3.13

Operating System

No response

@nielsdos
Copy link
Member

I agree

@nielsdos
Copy link
Member

I'm willing to look into this. It's a bit annoying because the polling API does not give us an updated timeout value, so we need to track it ourselves. I thought about using select() instead of poll(), but only on Linux does that actually update the timeout value :(

@bukka
Copy link
Member

bukka commented Nov 24, 2024

Was just fixing something similar: #16606

@nielsdos
Copy link
Member

Maybe, such a wrapper around the poll functionality should live in a general place

@bukka
Copy link
Member

bukka commented Nov 24, 2024

Yeah it could be useful. I see that it's actually timeout reduction does not seems to be always reduced by streams either - EINTR is handled but looks like timeout is not always handled ( e.g.

do {
retval = php_pollfd_for(sock->socket, POLLOUT, ptimeout);
if (retval == 0) {
sock->timeout_event = 1;
break;
}
if (retval > 0) {
/* writable now; retry */
goto retry;
}
err = php_socket_errno();
} while (err == EINTR);
).

We have been discussing new polling API for some time so this is something that should be there for sure. But even having some common helpers in the meantime would be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants