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

Added a note on failed handles not being counted by curl_multi_perform. #4446

Closed
wants to merge 1 commit into from

Conversation

akashihi
Copy link
Contributor

@akashihi akashihi commented Oct 1, 2019

No description provided.

Copy link
Member

@bagder bagder left a comment

Thanks for your help in improving the docs!

@@ -98,7 +98,12 @@ period for your select() calls.
one of its input arguments, and by reading that you can figure out when all
the transfers in the multi handles are done. 'done' does not mean
successful. One or more of the transfers may have failed. Tracking when this
number changes, you know when one or more transfers are done.
number changes, you know when one or more transfers are done. Please note, that
transfer may fail immediately, especially in case you are adding/re-using easy
Copy link
Member

@bagder bagder Oct 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. "a transfer" or "transfers" (not "transfer")
  2. Why is this likely to happen "especially" in the case you reuse handles? In my view, transfers are always roughly equally likely to fail.

Copy link
Contributor Author

@akashihi akashihi Oct 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Fixed
  2. Well, you are right, fixed.

transfer may fail immediately, especially in case you are adding/re-using easy
handles into already running multi handle. At the same time newly added
transfer is not guaranteed to start immediately. In both cases
\fIcurl_multi_perform(3)\fP will not count that handle as running transfer,
Copy link
Member

@bagder bagder Oct 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect. The number of "running handles" is increased by curl_multi_add_handle so a slow start doesn't result in a lower number. The number is however decreased when the transfer is complete, which can happen at any point and thus already in the fist call to curl_multi_perform().

Copy link
Contributor Author

@akashihi akashihi Oct 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rephrased to be more precise.

Copy link
Member

@bagder bagder Oct 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about instead trying to not duplicate what the curl_multi_perform.3 man page already says ? We can instead rather remove the (possibly slightly misleading) sentence "Tracking when this number changes, you know when one or more transfers are done."

Copy link
Contributor Author

@akashihi akashihi Oct 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

bagder
bagder approved these changes Oct 3, 2019
@bagder
Copy link
Member

@bagder bagder commented Oct 3, 2019

Thanks!

@lock lock bot locked as resolved and limited conversation to collaborators Jan 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants