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

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
Labels
documentation libcurl API

Comments

@hamstergene
Copy link

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
Contributor

dfandrich commented Dec 27, 2019 via email

@hamstergene
Copy link
Author

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
@jay
multi: Change curl_multi_wait/poll to error on negative timeout
8551a37 
- 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 jay mentioned this issue Dec 28, 2019
Closed
@jay
Copy link
Member

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 added a commit to jay/curl that referenced this issue Dec 28, 2019
@jay
multi: Change curl_multi_wait/poll to error on negative timeout
69e503d 
- 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
@jay
multi: Change curl_multi_wait/poll to error on negative timeout
b473d5e 
- 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 jay closed this as completed in b700662 Jan 12, 2020
@tomalakgeretkal
Copy link

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
@leodido @fntlnz
fix(userspace/libsinps/container_engine): curl_multi_wait timeout mus…
f0fc3dd 
…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 leodido mentioned this issue Apr 9, 2020
Merged
leodido added a commit to draios/sysdig that referenced this issue Apr 9, 2020
@leodido @fntlnz
fix(userspace/libsinps/container_engine): curl_multi_wait timeout mus…
a259b4b 
…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.
Assignees
No one assigned
Labels
documentation libcurl API
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

multi: Change curl_multi_wait/poll to error on negative timeout jay/curl
5 participants
@bagder @dfandrich @jay @hamstergene @tomalakgeretkal