Copilot commented on code in PR #3930:
URL: https://github.com/apache/thrift/pull/3930#discussion_r4099058236
##########
lib/netstd/Thrift/Transport/Client/THttpTransport.cs:
##########
@@ -293,10 +354,153 @@ protected override void Dispose(bool disposing)
{
_inputStream?.Dispose();
_outputStream?.Dispose();
+ _response?.Dispose();
+ _response = null;
+ _responseReadTimeout?.Dispose();
+ _responseReadTimeout = null;
_httpClient?.Dispose();
}
}
_isDisposed = true;
}
+
+public ValueTask<TTransport> CreatePerCallTransportAsync(CancellationToken
cancellationToken = default)
+ {
+ cancellationToken.ThrowIfCancellationRequested();
+ return new ValueTask<TTransport>(new THttpPerCallTransport(this));
+ }
+
+ internal static async Task<Stream> ReadAsStreamAsync(HttpContent
content, CancellationToken cancellationToken)
+ {
+#if NET5_0_OR_GREATER
+ return await content.ReadAsStreamAsync(cancellationToken);
+#else
+ var readTask = content.ReadAsStreamAsync();
+ if (readTask.IsCompleted || !cancellationToken.CanBeCanceled)
+ {
+ return await readTask;
+ }
+
+ // Observe a later failure if cancellation wins the race and the
response is disposed.
+ readTask.ContinueWith(
+ task => { var ignored = task.Exception; },
+ CancellationToken.None,
+ TaskContinuationOptions.OnlyOnFaulted |
TaskContinuationOptions.ExecuteSynchronously,
+ TaskScheduler.Default);
+
+ var cancellationTask = new TaskCompletionSource<bool>();
+ using (cancellationToken.Register(() =>
cancellationTask.TrySetResult(true)))
+ {
+ if (await Task.WhenAny(readTask, cancellationTask.Task) !=
readTask)
+ {
+ throw new OperationCanceledException(cancellationToken);
+ }
+ }
+
+ return await readTask;
+#endif
+ }
+
+ /// <summary>
+ /// Sends the given request stream to the server and returns the HTTP
response.
+ /// </summary>
+ /// <param name="request">The request stream to send.</param>
+ /// <param name="cancellationToken">The cancellation token to cancel
the operation.</param>
+ /// <returns>The HTTP response message.</returns>
+ /// <exception cref="TTransportException">Thrown if an error occurs
during the request.</exception>
+ internal async Task<HttpResponseMessage> SendAsync(Stream request,
CancellationToken cancellationToken)
+ {
+ // Snapshot the field locally: Close()/Dispose() running
concurrently on the parent
+ // transport can null out or dispose _httpClient while a per-call
transport is in flight.
+ var httpClient = _httpClient ?? throw new
TTransportException(TTransportException.ExceptionType.NotOpen,
+ "The transport has been closed.");
Review Comment:
This snapshot only prevents a null dereference; it does not keep the
`HttpClient` usable after `Close`/`Dispose` disposes the same instance. A close
racing with `SendAsync` can either turn an in-flight call into an unexpected
`NotOpen` error or let the call return and publish `_inputStream`, `_response`,
and `_responseReadTimeout` after `Close` has already cleaned up, leaking those
resources. Synchronize parent lifecycle with in-flight sends or dispose/reject
results that complete after closure.
--
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]
