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

Jump to bottom

multi_wait() ignores Curl_poll() errors #8921

Closed
piru opened this issue May 26, 2022 · 3 comments
Closed

multi_wait() ignores Curl_poll() errors #8921

piru opened this issue May 26, 2022 · 3 comments
Labels
libcurl API

Comments

@piru
Copy link

piru commented May 26, 2022

multi_wait() ignores the potential error from Curl_poll() at

curl/lib/multi.c

Line 1315 in d67f41a

If Curl_poll() returns immediately the function will then return CURLM_OK right away without performing any waiting. This will lead curl_multi_poll()/curl_multi_wait() to return immediately as well. Effectively this will lead to a busyloop as there is no status returned that would lead to terminating a loop of a typical curl_multi_perform() + curl_multi_poll() app, at least if the app doesn't bail out once the connections are completed (still_running == 0 doesn't lead to loop termination).

I did this

Had condition where poll() (or select() if using poll() emulation) return a permanent error without waiting.

I expected the following

curl_multi_poll() / curl_multi_wait() to return an error if Curl_poll() fails.

The question is what error to return though as none of the CURLM_ errors seem to fit the bill.

curl/libcurl version

7.83.1

operating system

This is a generic issue, but it is likely more likely to get triggered on specific platforms.
@bagder
Copy link
Member

bagder commented May 27, 2022

I don't think all poll/select errors are permanent though, are they? Should all underlying errors make curl_multi_poll()/wait() return error?

@piru
Copy link
Author

piru commented May 27, 2022 

ERRORS
       EFAULT fds  points  outside the process's accessible address space.  The array given as
              argument was not contained in the calling program's address space.

       EINTR  A signal occurred before any requested event; see signal(7).

       EINVAL The nfds value exceeds the RLIMIT_NOFILE value.

       EINVAL (ppoll()) The timeout value expressed in *ip is invalid (negative).

       ENOMEM Unable to allocate memory for kernel data structures.

Indeed, EINTR probably should not be considered fatal.

@piru
Copy link
Author

piru commented May 27, 2022

lib/easy.c wait_or_timeout() seems to have the same issue btw

curl/lib/easy.c

Line 509 in a77d14d

pollrc = Curl_poll(fds, numfds, ev->ms);

lib/asyn-ares.c waitperform() as well. waitperform() result is not checked, which seems to be wrong as well.

curl/lib/asyn-ares.c

Line 342 in a77d14d

nfds = Curl_poll(pfd, num, timeout_ms);

bagder added a commit that referenced this issue Jun 5, 2022
@bagder
select: return error from "lethal" poll/select errors
4279b52 
Reported-by: Harry Sintonen
Fixes #8921
@bagder bagder mentioned this issue Jun 5, 2022
Closed
@bagder bagder closed this as completed in 5912da2 Jun 8, 2022
t-8ch added a commit to t-8ch/curl that referenced this issue Jul 3, 2022
@t-8ch
select: do not return fatal error on EINTR from poll()
9f0a904 
The same was done for select() in 5912da2
but poll() was missed.

Downstream report: https://bugs.archlinux.org/task/75201
See curl#8921 and curl#8961
@t-8ch t-8ch mentioned this issue Jul 3, 2022
Closed
jay pushed a commit that referenced this issue Jul 4, 2022
@t-8ch @jay
select: do not return fatal error on EINTR from poll()
52e8221 
The same was done for select() in 5912da2 but poll() was missed.

Bug: https://bugs.archlinux.org/task/75201
Reported-by: Alexandre Bury (gyscos at archlinux)

Ref: #8921
Ref: #8961
Ref: 5912da25#r77584294

Closes #9091
@westonpace westonpace mentioned this issue May 15, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
libcurl API
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

select: return error from "lethal" poll/select errors curl/curl
2 participants
@bagder @piru