Skip to content

Commit d6f5061

Browse files
committed
Attempt to fix still running write while disposing the stream in case of a cancellation
1 parent e4c2f17 commit d6f5061

File tree

1 file changed

+30
-13
lines changed

1 file changed

+30
-13
lines changed

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ internal sealed class Http3RequestStream : IHttpStreamHeadersHandler, IAsyncDisp
3131
private TaskCompletionSource<bool>? _expect100ContinueCompletionSource; // True indicates we should send content (e.g. received 100 Continue).
3232
private bool _disposed;
3333
private readonly CancellationTokenSource _requestBodyCancellationSource;
34+
private Task? _sendRequestTask; // Set with SendContentAsync, must be awaited before QuicStream.DisposeAsync();
35+
private Task? _readResponseTask; // Set with ReadResponseAsync, must be awaited before QuicStream.DisposeAsync();
3436

3537
// Allocated when we receive a :status header.
3638
private HttpResponseMessage? _response;
@@ -107,9 +109,25 @@ public async ValueTask DisposeAsync()
107109
{
108110
_disposed = true;
109111
AbortStream();
112+
// We aborted both sides, thus both task should unblock and should be finished before disposing the QuicStream.
113+
await AwaitUnfinished(_sendRequestTask).ConfigureAwait(false);
114+
await AwaitUnfinished(_readResponseTask).ConfigureAwait(false);
110115
await _stream.DisposeAsync().ConfigureAwait(false);
111116
DisposeSyncHelper();
112117
}
118+
119+
static async ValueTask AwaitUnfinished(Task? task)
120+
{
121+
if (task is not null && !task.IsCompleted)
122+
{
123+
try
124+
{
125+
await task.ConfigureAwait(false);
126+
}
127+
catch // Exceptions from both tasks are logged via _connection.LogException() in case they're not awaited in SendAsync, so the exception can be ignored here.
128+
{ }
129+
}
130+
}
113131
}
114132

115133
private void DisposeSyncHelper()
@@ -158,52 +176,51 @@ public async Task<HttpResponseMessage> SendAsync(CancellationToken cancellationT
158176
await FlushSendBufferAsync(endStream: _request.Content == null, _requestBodyCancellationSource.Token).ConfigureAwait(false);
159177
}
160178

161-
Task sendContentTask;
162179
if (_request.Content != null)
163180
{
164-
sendContentTask = SendContentAsync(_request.Content!, _requestBodyCancellationSource.Token);
181+
_sendRequestTask = SendContentAsync(_request.Content!, _requestBodyCancellationSource.Token);
165182
}
166183
else
167184
{
168-
sendContentTask = Task.CompletedTask;
185+
_sendRequestTask = Task.CompletedTask;
169186
}
170187

171188
// In parallel, send content and read response.
172189
// Depending on Expect 100 Continue usage, one will depend on the other making progress.
173-
Task readResponseTask = ReadResponseAsync(_requestBodyCancellationSource.Token);
190+
_readResponseTask = ReadResponseAsync(_requestBodyCancellationSource.Token);
174191
bool sendContentObserved = false;
175192

176193
// If we're not doing duplex, wait for content to finish sending here.
177194
// If we are doing duplex and have the unlikely event that it completes here, observe the result.
178195
// See Http2Connection.SendAsync for a full comment on this logic -- it is identical behavior.
179-
if (sendContentTask.IsCompleted ||
196+
if (_sendRequestTask.IsCompleted ||
180197
_request.Content?.AllowDuplex != true ||
181-
await Task.WhenAny(sendContentTask, readResponseTask).ConfigureAwait(false) == sendContentTask ||
182-
sendContentTask.IsCompleted)
198+
await Task.WhenAny(_sendRequestTask, _readResponseTask).ConfigureAwait(false) == _sendRequestTask ||
199+
_sendRequestTask.IsCompleted)
183200
{
184201
try
185202
{
186-
await sendContentTask.ConfigureAwait(false);
203+
await _sendRequestTask.ConfigureAwait(false);
187204
sendContentObserved = true;
188205
}
189206
catch
190207
{
191-
// Exceptions will be bubbled up from sendContentTask here,
192-
// which means the result of readResponseTask won't be observed directly:
208+
// Exceptions will be bubbled up from _sendRequestTask here,
209+
// which means the result of _readResponseTask won't be observed directly:
193210
// Do a background await to log any exceptions.
194-
_connection.LogExceptions(readResponseTask);
211+
_connection.LogExceptions(_readResponseTask);
195212
throw;
196213
}
197214
}
198215
else
199216
{
200217
// Duplex is being used, so we can't wait for content to finish sending.
201218
// Do a background await to log any exceptions.
202-
_connection.LogExceptions(sendContentTask);
219+
_connection.LogExceptions(_sendRequestTask);
203220
}
204221

205222
// Wait for the response headers to be read.
206-
await readResponseTask.ConfigureAwait(false);
223+
await _readResponseTask.ConfigureAwait(false);
207224

208225
Debug.Assert(_response != null && _response.Content != null);
209226
// Set our content stream.

0 commit comments

Comments
 (0)