Copilot commented on code in PR #3978:
URL: https://github.com/apache/thrift/pull/3978#discussion_r4135939686
##########
lib/netstd/Thrift/Transport/Client/THttpTransport.cs:
##########
@@ -278,25 +458,350 @@ public override async Task FlushAsync(CancellationToken
cancellationToken)
}
finally
{
- _outputStream = new MemoryStream();
- ResetMessageSizeAndConsumedBytes();
+ inputStream?.Dispose();
+ response?.Dispose();
+ responseReadTimeout?.Dispose();
+ operationCts?.Dispose();
+ request?.Dispose();
+ if (request == null)
+ {
+ contentStream?.Dispose();
+ }
+
+ lock (_lifecycleLock)
+ {
+ _flushInProgress--;
+ if (_lifecycleOpen)
+ {
+ _outputStream = new MemoryStream();
+ }
+ else
+ {
+ outputStream.Dispose();
+ _outputStream = null;
+ }
+ ResetMessageSizeAndConsumedBytes();
+ }
}
}
// IDisposable
protected override void Dispose(bool disposing)
{
- if (!_isDisposed)
+ if (_isDisposed)
+ {
+ return;
+ }
+
+ if (disposing)
+ {
+ Close();
+ }
+ _isDisposed = true;
+ }
+
+ /// <inheritdoc />
+ public ValueTask<TTransport>
CreatePerCallTransportAsync(CancellationToken cancellationToken = default)
+ {
+ cancellationToken.ThrowIfCancellationRequested();
+ lock (_lifecycleLock)
{
- if (disposing)
+ if (!_lifecycleOpen || _httpClient == null)
{
- _inputStream?.Dispose();
- _outputStream?.Dispose();
- _httpClient?.Dispose();
+ throw new
TTransportException(TTransportException.ExceptionType.NotOpen,
+ "The transport has been closed.");
}
+
+ var transport = new THttpPerCallTransport(this);
+ _perCallTransports.Add(transport);
+ return new ValueTask<TTransport>(transport);
+ }
+ }
+
+ internal object LifecycleLock => _lifecycleLock;
+
+ internal bool IsLifecycleOpen
+ {
+ get
+ {
+ lock (_lifecycleLock)
+ {
+ return _lifecycleOpen && _httpClient != null;
+ }
+ }
+ }
+
+ internal void RemovePerCallTransport(THttpPerCallTransport transport)
+ {
+ lock (_lifecycleLock)
+ {
+ _perCallTransports.Remove(transport);
+ }
+ }
+
+ 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;
+ }
Review Comment:
The newly added legacy cancellation path is not exercised by the tests:
`Thrift.Tests.csproj` targets only `net10.0`, so it compiles the
`NET5_0_OR_GREATER` branch, while the `netstandard2.0` project is compile-only.
This leaves the continuation that disposes a stream after canceled acquisition
unverified on the frameworks that use it. Please add a runtime-targeted test
for this branch covering cancellation followed by eventual stream
completion/disposal.
--
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]
