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

easy: fix curl_easy_upkeep for shared connection caches #12677

Closed
wants to merge 1 commit into from

Conversation

jay
Copy link
Member

@jay jay commented Jan 10, 2024

  • Predict which connection cache is or will be used by the easy handle and perform connection upkeep on that cache.

This change allows curl_easy_upkeep to be effective on easy handles that are using a shared connection cache, either from a user created shared connection cache or a user created multi which has its own shared connection cache.

Prior to this change curl_easy_upkeep would upkeep the connection cache for the easy handle only if that cache was from the multi owned by the easy handle (ie curl_easy_perform was previously called and there's a connection cache exclusive to the easy handle in
data->multi_easy->conn_cache).

Ref: https://curl.se/mail/lib-2024-01/0016.html

Closes #xxxx


It's probably a matter of perspective whether this is a bug or not. I didn't find anything to indicate that the function wasn't intended to work with shared connection caches.

Note this change will allow calling on an easy handle that is part of a user created multi, therefore a user of that interface might use this function and not expect it to block? It's unclear to me whether or not the function could actually block.

/cc @maxdymond

lib/easy.c Outdated Show resolved Hide resolved
@bagder
Copy link
Member

bagder commented Jun 14, 2024

what's the status here @jay ?

@jay
Copy link
Member Author

jay commented Jun 18, 2024

This change makes it possible to call easy upkeep on easy handles in multi handles and I can't say for certain it won't block in that case. Also I changed it so easy upkeep can't be called from a callback since I can't say for certain a callback won't be called during the upkeep.

If nobody has a problem with either of those two things I'll land it.

@bagder
Copy link
Member

bagder commented Jul 1, 2024

Predict which connection cache is or will be used by the easy handle and perform connection upkeep on that cache.

I have a minor issue with this wording here. Predicting means guessing how something will be or happen in a future. This code does not predict, it determines or possibly decides which connection cache to check using heuristics.

- Determine which connection cache is or will be used by the easy handle
  and perform connection upkeep on that cache.

This change allows curl_easy_upkeep to be effective on easy handles that
are using a shared connection cache, either from a user created shared
connection cache or a user created multi which has its own shared
connection cache.

Prior to this change curl_easy_upkeep would upkeep the connection cache
for the easy handle only if that cache was from the multi owned by the
easy handle (ie curl_easy_perform was previously called and there's a
connection cache exclusive to the easy handle in
data->multi_easy->conn_cache).

Ref: https://curl.se/mail/lib-2024-01/0016.html

Closes #xxxx
@jay
Copy link
Member Author

jay commented Jul 19, 2024

I have a minor issue with this wording here. Predicting means guessing how something will be or happen in a future. This code does not predict, it determines or possibly decides which connection cache to check using heuristics.

Ok I changed it to say determine and I added to the documentation to explain that upkeep on a handle with a shared connection cache will perform upkeep on all the connections in that cache.

@jay jay closed this in 573aaec Aug 4, 2024
@jay jay deleted the fix_upkeep branch August 4, 2024 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants