[PATCH] D121720: [Debuginfod] Don't depend on Content-Length.

[Noah Shutty via Phabricator via llvm-commits]

noajshu added a comment.

In D121720#3383895 <https://reviews.llvm.org/D121720#3383895>, @mysterymath wrote:

> In D121720#3383783 <https://reviews.llvm.org/D121720#3383783>, @noajshu wrote:
>
>> Indeed, an application can use the same `HTTPClient` from multiple threads; this new design prevents us from learning the individual response codes for concurrent requests.
>
> If I'm interpreting the libcurl docs correctly, it doesn't seem like it's legal to issue multiple curl_easy_performs on the same Curl handle: https://curl.se/libcurl/c/curl_easy_perform.html
> HTTPClient should still be usable in a multi-threaded fashion, but you'd need one HTTPClient per thread.

Ah, thank you for correcting me. I agree there should be one `HTTPClient` and Curl handle per thread.

>> If you agree, I think you can just un-delete the `handleResponseCode` declaration + definition and its invocation inside `perform()`.
>
> I initially tried to do this, but as previously implemented, handleResponseCode only gets called after the full data transfer is complete, which is too late to decide whether or not to create a cache file.
> We could parse the response code itself in handleHeaderLine, but that seemed a bit heavyweight, and I wasn't sure exactly how it'd interact with CURLOPT_FOLLOWLOCATION.

If you like, you could set a status code member of the Handler when you check to determine whether to create a cache file. In this case you could even have the `HTTPClient::statusCode()` be a private method and make the Handler a friend.
On the other hand, since the multithreaded access is not an issue, perhaps it's not worth the trouble to make the `HTTPClient` interface stateless.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121720/new/

https://reviews.llvm.org/D121720