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

`curl_multi_poll` hangs forever on negative timeout #4763

Closed
hamstergene opened this issue Dec 27, 2019 · 4 comments
Closed

`curl_multi_poll` hangs forever on negative timeout #4763

hamstergene opened this issue Dec 27, 2019 · 4 comments

Comments

@hamstergene
Copy link

@hamstergene hamstergene commented Dec 27, 2019

This program hangs forever

#include <curl/curl.h>

int main(int narg, char**argv) {
    curl_global_init( CURL_GLOBAL_ALL );
    CURLM* mh = curl_multi_init();
    curl_multi_poll( mh, 0, 0, /*timeout_ms*/-1, 0 );
}

I expected the following

The function interface takes signed integer (int) it must be ready to handle negative numbers.

curl/libcurl version

7.68.0-20191218

operating system

macOS 10.14.6

@dfandrich
Copy link
Collaborator

@dfandrich dfandrich commented Dec 27, 2019

@hamstergene
Copy link
Author

@hamstergene hamstergene commented Dec 27, 2019

Positive and zero are also undocumented. The is simply no documentation on which values are valid. One has to document something first before claiming that everything else is undocumented.

jay added a commit to jay/curl that referenced this issue Dec 28, 2019
- Add new error CURLM_BAD_FUNCTION_ARGUMENT and return that error when
  curl_multi_wait/poll is passed timeout param < 0.

Prior to this change passing a negative value to curl_multi_wait/poll
such as -1 could cause the function to wait forever.

Reported-by: hamstergene@users.noreply.github.com

Fixes curl#4763

Closes #xxxx
@jay
Copy link
Member

@jay jay commented Dec 28, 2019

It's implied though we should error if negative. I couldn't find a good error code for it so I've added one in #4765 and if approved that will address this issue in the release after next.

@jay jay added the libcurl API label Dec 28, 2019
jay added a commit to jay/curl that referenced this issue Dec 28, 2019
- Add new error CURLM_BAD_FUNCTION_ARGUMENT and return that error when
  curl_multi_wait/poll is passed timeout param < 0.

Prior to this change passing a negative value to curl_multi_wait/poll
such as -1 could cause the function to wait forever.

Reported-by: hamstergene@users.noreply.github.com

Fixes curl#4763

Closes #xxxx
jay added a commit to jay/curl that referenced this issue Dec 28, 2019
- Add new error CURLM_BAD_FUNCTION_ARGUMENT and return that error when
  curl_multi_wait/poll is passed timeout param < 0.

Prior to this change passing a negative value to curl_multi_wait/poll
such as -1 could cause the function to wait forever.

Reported-by: hamstergene@users.noreply.github.com

Fixes curl#4763

Closes #xxxx
@bagder bagder added the documentation label Jan 4, 2020
@jay jay closed this in b700662 Jan 12, 2020
@tomalakgeretkal
Copy link

@tomalakgeretkal tomalakgeretkal commented Feb 16, 2020

I don't understand why we cannot accept -1 for "forever" so that this functions like POSIX poll(). It's easy to make it behave properly with expiry timeouts: I'd submitted a PR for it before realising that this new validation had been added, which breaks my fix. #4932

leodido added a commit to draios/sysdig that referenced this issue Apr 9, 2020
…t be a positive value (to do not error on libcurl >= 7.69.0)

When compiling sysdig with libcurl >= 7.69.0 `timeout_ms` equal to `-1` causes `curl_multi_wait` to return an error ([ref](https://github.com/jay/curl/blob/master/lib/multi.c#L1056-L1057)). This new behaviour has been introduced with [0](https://github.com/jay/curl/commit/b700662b1c77c8af7e290538f748b71d75a79ae7.patch).

More details on the reasoning behind this choice can be found in this [issue](curl/curl#4763).

So, to obtain the same behavior disregarding the libcurl version sysdig has been built with, this patch increases the `timeout_ms` value to `1000`.

Please notice that this does not mean it will use `1000` as timeout value since internally `curl_multi_wait` sets the actual timeout value to `timeout_internal` in case this is less than the passed `timeout_ms` argument.

Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
leodido added a commit to draios/sysdig that referenced this issue Apr 9, 2020
…t be a positive value (to do not error on libcurl >= 7.69.0)

When compiling sysdig with libcurl >= 7.69.0 `timeout_ms` equal to `-1` causes `curl_multi_wait` to return an error ([ref](https://github.com/jay/curl/blob/master/lib/multi.c#L1056-L1057)). This new behaviour has been introduced with [0](https://github.com/jay/curl/commit/b700662b1c77c8af7e290538f748b71d75a79ae7.patch).

More details on the reasoning behind this choice can be found in this [issue](curl/curl#4763).

So, to obtain the same behavior disregarding the libcurl version sysdig has been built with, this patch increases the `timeout_ms` value to `1000`.

Please notice that this does not mean it will use `1000` as timeout value since internally `curl_multi_wait` sets the actual timeout value to `timeout_internal` in case this is less than the passed `timeout_ms` argument.

Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants
You can’t perform that action at this time.