Skip to content

Commit d139d55

Browse files
author
ladeak
committed
Resolving race condition when writing end stream for HEADER frame: race between hascontent data and writing the header. endstream is now always written separately.
1 parent 29f3cf0 commit d139d55

File tree

2 files changed

+12
-11
lines changed

2 files changed

+12
-11
lines changed

src/CHttpServer/CHttpServer/Http2ResponseWriter.cs

+5-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System.Buffers;
22
using System.IO;
3+
using System.IO.Pipelines;
34
using System.Text;
45
using System.Threading.Channels;
56
using CHttpServer.System.Net.Http.HPack;
@@ -55,6 +56,7 @@ public async Task RunAsync(CancellationToken token)
5556
await WriteTrailersAsync(request.Stream);
5657
}
5758
}
59+
// TODO propagate the exception to the caller
5860
catch (OperationCanceledException)
5961
{
6062
// Channel is closed by the connection.
@@ -88,7 +90,8 @@ internal void ScheduleWriteTrailers(Http2Stream http2Stream) =>
8890
private async Task WriteDataAsync(Http2Stream stream)
8991
{
9092
long writtenCount = 0;
91-
while (stream.ResponseContent.TryRead(out var readResult))
93+
ReadResult readResult;
94+
while (stream.ResponseContent.TryRead(out readResult))
9295
{
9396
var responseContent = readResult.Buffer;
9497
if (readResult.IsCanceled || (readResult.IsCompleted && responseContent.IsEmpty))
@@ -134,8 +137,7 @@ private async Task WriteHeadersAsync(Http2Stream stream)
134137
throw new InvalidOperationException("Header too large");
135138
totalLength += writtenLength;
136139
}
137-
138-
_frameWriter.WriteResponseHeader(stream.StreamId, _buffer.AsMemory(0, totalLength), endStream: !stream.HasResponseContent);
140+
_frameWriter.WriteResponseHeader(stream.StreamId, _buffer.AsMemory(0, totalLength), endStream: false);
139141
await _frameWriter.FlushAsync();
140142
}
141143

src/CHttpServer/CHttpServer/Http2Stream.cs

+7-8
Original file line numberDiff line numberDiff line change
@@ -232,8 +232,6 @@ IHeaderDictionary IHttpResponseFeature.Headers
232232

233233
public HeaderCollection ResponseHeaders => _responseHeaders ??= new();
234234

235-
internal bool HasResponseContent => _responseContentPipeWriter.HasData;
236-
237235
public void DisableBuffering()
238236
{
239237
throw new NotImplementedException();
@@ -273,8 +271,13 @@ public async Task StartAsync(CancellationToken cancellationToken = default)
273271
_responseWritingTask = StartResponseAsync(_cts.Token);
274272
}
275273

276-
public Task CompleteAsync() =>
277-
_responseWritingTask ?? StartResponseAsync();
274+
public async Task CompleteAsync()
275+
{
276+
var task = _responseWritingTask;
277+
if (task == null)
278+
await StartAsync();
279+
await _responseWritingTask!;
280+
}
278281

279282
private async Task StartResponseAsync(CancellationToken cancellationToken = default)
280283
{
@@ -312,18 +315,14 @@ public class Http2StreamPipeWriter(PipeWriter writer, Action writeStartedCallbac
312315
{
313316
private readonly PipeWriter _writer = writer;
314317
private readonly Action _writeStartedCallback = writeStartedCallback;
315-
private volatile bool _hasData;
316318
private volatile bool _completed;
317319
private long _unflushedBytes;
318320

319-
public bool HasData => _hasData;
320-
321321
public bool IsCompleted => _completed;
322322

323323
public override void Advance(int bytes)
324324
{
325325
_writer.Advance(bytes);
326-
_hasData = true;
327326
_unflushedBytes += bytes;
328327
}
329328

0 commit comments

Comments
 (0)