Skip to content

Commit 5d2f669

Browse files
ladeakladeak
authored
Merge pull request #53 from ladeak/flaky_tests
Addressing flaky tests
2 parents ff9cf57 + 3c7df7f commit 5d2f669

6 files changed

Lines changed: 175 additions & 32 deletions

File tree

‎src/CHttpServer/CHttpServer/Http3/Http3FramingStreamWriter.cs‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public override async ValueTask<FlushResult> WriteAsync(ReadOnlyMemory<byte> sou
142142
var frameHeaderLength = PrepareFrameHeader(buffer.AsSpan(), source.Length, _frameType);
143143
source.CopyTo(buffer.AsMemory(frameHeaderLength));
144144
await _responseStream.WriteAsync(buffer.AsMemory(0..(source.Length + frameHeaderLength)), cancellationToken);
145-
ArrayPool<byte>.Shared.Return(buffer);
145+
ArrayPool<byte>.Shared.Return(buffer, true);
146146
}
147147
else
148148
{
@@ -208,11 +208,6 @@ public override async ValueTask<FlushResult> FlushAsync(CancellationToken cancel
208208

209209
private async Task<FlushResult> FlushAllSegmentsAsync(int startOffset, CancellationToken localToken)
210210
{
211-
if (!_currentSegment.IsEmpty)
212-
_segments.Add(_currentSegment);
213-
else if (_currentSegment.IsAllocated)
214-
_currentSegment = _currentSegment with { Used = Memory<byte>.Empty };
215-
216211
// First segment handled with offset.
217212
var memory = _segments[0];
218213
Debug.Assert(!memory.IsEmpty);
@@ -227,6 +222,12 @@ private async Task<FlushResult> FlushAllSegmentsAsync(int startOffset, Cancellat
227222
await _responseStream.WriteAsync(memory.Used, localToken);
228223
_memoryPool.Return(memory.Reference, true);
229224
}
225+
226+
// Last segment (the _currentSegment is not returned to the memory pool.
227+
if (!_currentSegment.IsEmpty)
228+
await _responseStream.WriteAsync(_currentSegment.Used, localToken);
229+
_currentSegment = _currentSegment with { Used = Memory<byte>.Empty };
230+
230231
_segments.Clear();
231232
_unflushedBytes = 0;
232233
_responseStream.Flush();
@@ -263,24 +264,27 @@ public void Flush()
263264

264265
private void FlushAllSegments(int startOffset)
265266
{
266-
if (!_currentSegment.IsEmpty)
267-
_segments.Add(_currentSegment);
268-
else if (_currentSegment.IsAllocated)
269-
_currentSegment = _currentSegment with { Used = Memory<byte>.Empty };
270-
267+
// First segment handled with offset.
271268
var source = CollectionsMarshal.AsSpan(_segments);
272269
ref var initialMemory = ref source[0];
273270
Debug.Assert(!initialMemory.IsEmpty);
274271
_responseStream.Write(initialMemory.Used.Span[startOffset..]);
275272
_memoryPool.Return(initialMemory.Reference, true);
276273

274+
// Remaining segments.
277275
for (int i = 1; i < _segments.Count; i++)
278276
{
279277
ref var memory = ref source[i];
280278
if (memory.Used.Length > 0)
281279
_responseStream.Write(memory.Used.Span);
282280
_memoryPool.Return(memory.Reference, true);
283281
}
282+
283+
// Last segment (the _currentSegment is not returned to the memory pool.
284+
if (!_currentSegment.IsEmpty)
285+
_responseStream.Write(_currentSegment.Used.Span);
286+
_currentSegment = _currentSegment with { Used = Memory<byte>.Empty };
287+
284288
_segments.Clear();
285289
_unflushedBytes = 0;
286290
_responseStream.Flush();
@@ -335,15 +339,15 @@ private void ClearSegments(Span<Segment> source, bool clearCurrent = true)
335339
for (int i = 0; i < source.Length; i++)
336340
{
337341
ref var memory = ref source[i];
338-
if (memory.Reference.Length != 0)
342+
if (memory.IsAllocated)
339343
_memoryPool.Return(memory.Reference, true);
340344
}
341345
_unflushedBytes = 0;
342346
_segments.Clear();
343347
if (clearCurrent)
344348
{
345349
if (_currentSegment.IsAllocated)
346-
_memoryPool.Return(_currentSegment.Reference);
350+
_memoryPool.Return(_currentSegment.Reference, true);
347351
_currentSegment = new Segment();
348352
}
349353
else

‎src/CHttpServer/CHttpServer/Http3/Http3HeaderFramingStreamWriter.cs‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,6 @@ public override async ValueTask<FlushResult> FlushAsync(CancellationToken cancel
177177

178178
private async ValueTask<FlushResult> FlushAllSegmentsAsync(int startOffset, CancellationToken localToken)
179179
{
180-
if (!_currentSegment.IsEmpty)
181-
_segments.Add(_currentSegment);
182-
else if (_currentSegment.IsAllocated)
183-
_currentSegment = _currentSegment with { Used = Memory<byte>.Empty };
184-
185180
// First segment handled with offset.
186181
var memory = _segments[0];
187182
Debug.Assert(!memory.IsEmpty);
@@ -196,6 +191,12 @@ private async ValueTask<FlushResult> FlushAllSegmentsAsync(int startOffset, Canc
196191
await _responseStream.WriteAsync(memory.Used, localToken);
197192
_memoryPool.Return(memory.Reference, true);
198193
}
194+
195+
// Last segment (the _currentSegment is not returned to the memory pool.
196+
if (!_currentSegment.IsEmpty)
197+
await _responseStream.WriteAsync(_currentSegment.Used, localToken);
198+
_currentSegment = _currentSegment with { Used = Memory<byte>.Empty };
199+
199200
_segments.Clear();
200201
_unflushedBytes = 0;
201202
_responseStream.Flush();
@@ -225,24 +226,27 @@ public void Flush()
225226

226227
private void FlushAllSegments(int startOffset)
227228
{
228-
if (!_currentSegment.IsEmpty)
229-
_segments.Add(_currentSegment);
230-
else if (_currentSegment.IsAllocated)
231-
_currentSegment = _currentSegment with { Used = Memory<byte>.Empty };
232-
229+
// First segment handled with offset.
233230
var source = CollectionsMarshal.AsSpan(_segments);
234231
ref var initialMemory = ref source[0];
235232
Debug.Assert(!initialMemory.IsEmpty);
236233
_responseStream.Write(initialMemory.Used.Span[startOffset..]);
237234
_memoryPool.Return(initialMemory.Reference, true);
238235

236+
// Remaining segments.
239237
for (int i = 1; i < _segments.Count; i++)
240238
{
241239
ref var memory = ref source[i];
242240
if (memory.Used.Length > 0)
243241
_responseStream.Write(memory.Used.Span);
244242
_memoryPool.Return(memory.Reference, true);
245243
}
244+
245+
// Last segment (the _currentSegment is not returned to the memory pool.
246+
if (!_currentSegment.IsEmpty)
247+
_responseStream.Write(_currentSegment.Used.Span);
248+
_currentSegment = _currentSegment with { Used = Memory<byte>.Empty };
249+
246250
_segments.Clear();
247251
_unflushedBytes = 0;
248252
_responseStream.Flush();
@@ -305,7 +309,7 @@ private void ClearSegments(Span<Segment> source, bool clearCurrent = true)
305309
if (clearCurrent)
306310
{
307311
if (_currentSegment.IsAllocated)
308-
_memoryPool.Return(_currentSegment.Reference);
312+
_memoryPool.Return(_currentSegment.Reference, true);
309313
_currentSegment = new Segment();
310314
}
311315
else

‎tests/CHttpServer.Tests/CHttpServerIntegrationTests.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
using System.Net;
2-
using System.Net.Http.Json;
32
using System.Text;
43

54
namespace CHttpServer.Tests;
65

76
[Collection(nameof(VanilaCHttpServerIntegrationTests))]
8-
[CollectionDefinition(DisableParallelization = true)]
97
public class VanilaCHttpServerIntegrationTests : CHttpServerIntegrationTests, IClassFixture<TestServer>
108
{
119
private const int Port = 7222;
@@ -17,7 +15,6 @@ public VanilaCHttpServerIntegrationTests(TestServer testServer) : base(testServe
1715
}
1816

1917
[Collection(nameof(PriorityCHttpServerIntegrationTests))]
20-
[CollectionDefinition(DisableParallelization = true)]
2118
public class PriorityCHttpServerIntegrationTests : CHttpServerIntegrationTests, IClassFixture<TestServer>
2219
{
2320
private const int Port = 7223;

‎tests/CHttpServer.Tests/Http2ConnectionTests.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System.Buffers;
2-
using System.Diagnostics.CodeAnalysis;
32
using System.Net;
43
using System.Text;
54
using CHttpServer.System.Net.Http.HPack;
@@ -9,7 +8,6 @@
98

109
namespace CHttpServer.Tests;
1110

12-
[SuppressMessage("Usage", "xUnit1051:Calls to methods which accept CancellationToken should use TestContext.Current.CancellationToken")]
1311
public class Http2ConnectionTests
1412
{
1513
[Fact]

‎tests/CHttpServer.Tests/Http3/Http3FramingStreamWriterTests.cs‎

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public async Task GetSpan_Flush_WritesToStreamWithFrameHeader(int payloadLength,
3535

3636
for (int i = 0; i < payloadLength; i++)
3737
Assert.Equal((byte)i, result[i + headerLength]);
38-
38+
3939
sut.Complete();
4040
Assert.Equal(0, arrayPool.OutstandingBytes);
4141
}
@@ -549,6 +549,76 @@ public async Task WriteAsync_Writes_FrameType()
549549
Assert.Equal(expected, ms.ToArray());
550550
}
551551

552+
[Fact]
553+
public async Task ArrayPool_DoubleReturn_FlushAsync_Complete()
554+
{
555+
var sut = new Http3FramingStreamWriter(Stream.Null, 1, new TestArrayPool());
556+
sut.GetMemory(8192);
557+
sut.Advance(8100);
558+
sut.GetMemory(8192);
559+
sut.Advance(8100);
560+
// Flushes 2 segments.
561+
await sut.FlushAsync(TestContext.Current.CancellationToken);
562+
563+
sut.Complete(); // Should not double clear segments. If so, TestArrayPool will throw an exception.
564+
}
565+
566+
[Fact]
567+
public void ArrayPool_DoubleReturn_Flush_Complete()
568+
{
569+
var sut = new Http3FramingStreamWriter(Stream.Null, 1, new TestArrayPool());
570+
sut.GetMemory(8192);
571+
sut.Advance(8100);
572+
sut.GetMemory(8192);
573+
sut.Advance(8100);
574+
// Flushes 2 segments.
575+
sut.Flush();
576+
577+
sut.Complete(); // Should not double clear segments. If so, TestArrayPool will throw an exception.
578+
}
579+
580+
[Fact]
581+
public async Task ArrayPool_DoubleReturn_FlushAsync_CompleteAsync()
582+
{
583+
var sut = new Http3FramingStreamWriter(Stream.Null, 1, new TestArrayPool());
584+
sut.GetMemory(8192);
585+
sut.Advance(8100);
586+
sut.GetMemory(8192);
587+
sut.Advance(8100);
588+
// Flushes 2 segments.
589+
await sut.FlushAsync(TestContext.Current.CancellationToken);
590+
591+
await sut.CompleteAsync(); // Should not double clear segments. If so, TestArrayPool will throw an exception.
592+
}
593+
594+
[Fact]
595+
public async Task ArrayPool_DoubleReturn_Flush_CompleteAsync()
596+
{
597+
var sut = new Http3FramingStreamWriter(Stream.Null, 1, new TestArrayPool());
598+
sut.GetMemory(8192);
599+
sut.Advance(8100);
600+
sut.GetMemory(8192);
601+
sut.Advance(8100);
602+
// Flushes 2 segments.
603+
sut.Flush();
604+
605+
await sut.CompleteAsync(); // Should not double clear segments. If so, TestArrayPool will throw an exception.
606+
}
607+
608+
[Fact]
609+
public void ArrayPool_DoubleReturn_Flush_Reset()
610+
{
611+
var sut = new Http3FramingStreamWriter(Stream.Null, 1, new TestArrayPool());
612+
sut.GetMemory(8192);
613+
sut.Advance(8100);
614+
sut.GetMemory(8192);
615+
sut.Advance(8100);
616+
// Flushes 2 segments.
617+
sut.Flush();
618+
619+
sut.Reset(Stream.Null); // Should not double clear segments. If so, TestArrayPool will throw an exception.
620+
}
621+
552622
private class TestArrayPool : ArrayPool<byte>
553623
{
554624
private readonly ArrayPool<byte> _internalPool;
@@ -576,7 +646,7 @@ public override byte[] Rent(int minimumLength)
576646
public override void Return(byte[] array, bool clearArray = false)
577647
{
578648
_internalPool.Return(array, clearArray);
579-
_rentedArrays.Remove(array);
649+
Assert.True(_rentedArrays.Remove(array)); // Should not return an array twice.
580650
}
581651
}
582652
}

‎tests/CHttpServer.Tests/Http3/Http3HeaderFramingStreamWriterTests.cs‎

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,76 @@ public async Task WriteAsync_Writes_FrameType()
474474
Assert.Equal(expected, ms.ToArray());
475475
}
476476

477+
[Fact]
478+
public async Task ArrayPool_DoubleReturn_FlushAsync_Complete()
479+
{
480+
var sut = new Http3HeaderFramingStreamWriter(Stream.Null, new TestArrayPool());
481+
sut.GetMemory(8192);
482+
sut.Advance(8100);
483+
sut.GetMemory(8192);
484+
sut.Advance(8100);
485+
// Flushes 2 segments.
486+
await sut.FlushAsync(TestContext.Current.CancellationToken);
487+
488+
sut.Complete(); // Should not double clear segments. If so, TestArrayPool will throw an exception.
489+
}
490+
491+
[Fact]
492+
public void ArrayPool_DoubleReturn_Flush_Complete()
493+
{
494+
var sut = new Http3HeaderFramingStreamWriter(Stream.Null, new TestArrayPool());
495+
sut.GetMemory(8192);
496+
sut.Advance(8100);
497+
sut.GetMemory(8192);
498+
sut.Advance(8100);
499+
// Flushes 2 segments.
500+
sut.Flush();
501+
502+
sut.Complete(); // Should not double clear segments. If so, TestArrayPool will throw an exception.
503+
}
504+
505+
[Fact]
506+
public async Task ArrayPool_DoubleReturn_FlushAsync_CompleteAsync()
507+
{
508+
var sut = new Http3HeaderFramingStreamWriter(Stream.Null, new TestArrayPool());
509+
sut.GetMemory(8192);
510+
sut.Advance(8100);
511+
sut.GetMemory(8192);
512+
sut.Advance(8100);
513+
// Flushes 2 segments.
514+
await sut.FlushAsync(TestContext.Current.CancellationToken);
515+
516+
await sut.CompleteAsync(); // Should not double clear segments. If so, TestArrayPool will throw an exception.
517+
}
518+
519+
[Fact]
520+
public async Task ArrayPool_DoubleReturn_Flush_CompleteAsync()
521+
{
522+
var sut = new Http3HeaderFramingStreamWriter(Stream.Null, new TestArrayPool());
523+
sut.GetMemory(8192);
524+
sut.Advance(8100);
525+
sut.GetMemory(8192);
526+
sut.Advance(8100);
527+
// Flushes 2 segments.
528+
sut.Flush();
529+
530+
await sut.CompleteAsync(); // Should not double clear segments. If so, TestArrayPool will throw an exception.
531+
}
532+
533+
[Fact]
534+
public void ArrayPool_DoubleReturn_Flush_Reset()
535+
{
536+
var sut = new Http3HeaderFramingStreamWriter(Stream.Null, new TestArrayPool());
537+
sut.GetMemory(8192);
538+
sut.Advance(8100);
539+
sut.GetMemory(8192);
540+
sut.Advance(8100);
541+
// Flushes 2 segments.
542+
sut.Flush();
543+
544+
sut.Reset(Stream.Null); // Should not double clear segments. If so, TestArrayPool will throw an exception.
545+
}
546+
477547
private class TestArrayPool : ArrayPool<byte>
478548
{
479549
private readonly ArrayPool<byte> _internalPool;
@@ -501,7 +571,7 @@ public override byte[] Rent(int minimumLength)
501571
public override void Return(byte[] array, bool clearArray = false)
502572
{
503573
_internalPool.Return(array, clearArray);
504-
_rentedArrays.Remove(array);
574+
Assert.True(_rentedArrays.Remove(array));
505575
}
506576
}
507577
}

0 commit comments

Comments
 (0)