Copilot commented on code in PR #3930:
URL: https://github.com/apache/thrift/pull/3930#discussion_r4077231865
##########
lib/netstd/Thrift/Transport/Client/THttpTransport.cs:
##########
@@ -245,7 +245,15 @@ public override async Task FlushAsync(CancellationToken
cancellationToken)
{
contentStream.Headers.ContentType = ContentType ?? new
MediaTypeHeaderValue(@"application/x-thrift");
- var response = (await _httpClient.PostAsync(_uri,
contentStream, cancellationToken)).EnsureSuccessStatusCode();
+ using var request = new
HttpRequestMessage(HttpMethod.Post, _uri)
+ {
+ Content = contentStream
+ };
+
+ var response = (await _httpClient.SendAsync(
+ request,
+ HttpCompletionOption.ResponseHeadersRead,
+ cancellationToken)).EnsureSuccessStatusCode();
Review Comment:
Using `ResponseHeadersRead` here changes the existing shared transport from
fully buffering the response before returning, but the `HttpResponseMessage` is
only a local and is never retained for disposal. `_inputStream` is cleaned up
only on the next flush or close, so an abandoned or failed read can keep the
HTTP connection leased and exhaust the client's connection pool; retain/dispose
the response with the input stream, or keep the buffered completion mode for
this path.
##########
lib/netstd/Thrift/Transport/Client/THttpTransport.cs:
##########
@@ -245,7 +245,15 @@ public override async Task FlushAsync(CancellationToken
cancellationToken)
{
contentStream.Headers.ContentType = ContentType ?? new
MediaTypeHeaderValue(@"application/x-thrift");
- var response = (await _httpClient.PostAsync(_uri,
contentStream, cancellationToken)).EnsureSuccessStatusCode();
+ using var request = new
HttpRequestMessage(HttpMethod.Post, _uri)
+ {
+ Content = contentStream
+ };
+
+ var response = (await _httpClient.SendAsync(
+ request,
+ HttpCompletionOption.ResponseHeadersRead,
+ cancellationToken)).EnsureSuccessStatusCode();
Review Comment:
Using `ResponseHeadersRead` here changes the existing timeout boundary:
`HttpClient.Timeout` (configured by `ConnectTimeout`) stops governing the
response body once headers arrive, but the transport later reads that body with
only the caller token. A server that sends headers and then stalls the body can
therefore block indefinitely despite the configured timeout. Keep
`ResponseContentRead` for this path or apply an explicit linked timeout while
reading the body.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
