Skip to content

Commit 0b75617

Browse files
committed
Refactored InjectableTestOutputSink for teardown safety, preventing hangs and dropped logs.
Added bounded channels, cancellation tokens, and timed disposal for clean, non-blocking shutdown.
1 parent be6faa6 commit 0b75617

File tree

7 files changed

+192
-56
lines changed

7 files changed

+192
-56
lines changed

src/Serilog.Sinks.XUnit.Injectable/Abstract/IInjectableTestOutputSink.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,17 @@
88
namespace Serilog.Sinks.XUnit.Injectable.Abstract;
99

1010
/// <summary>
11-
/// A sink to direct Serilog output to the XUnit test output via dependency injection <para/>
12-
/// Use as a Singleton
11+
/// Serilog sink that writes to xUnit's <see cref="ITestOutputHelper"/> and/or an <see cref="IMessageSink"/>.
12+
/// Safe to dispose during/after test teardown without hangs. Use as a singleton.
1313
/// </summary>
1414
public interface IInjectableTestOutputSink : ILogEventSink, IAsyncDisposable, IDisposable
1515
{
16+
/// <summary>
17+
/// Optional early stop; disposing already does this. Call if you want the sink quiet
18+
/// before you flush Serilog.
19+
/// </summary>
20+
void Complete();
21+
1622
/// <summary>
1723
/// Call this as soon as you have a new instance of the testOutputHelper (usually at the beginning of a xUnit test
1824
/// class)
Lines changed: 159 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,125 +1,214 @@
11
using Serilog.Events;
22
using Serilog.Formatting.Display;
33
using Serilog.Sinks.XUnit.Injectable.Abstract;
4-
using System;
5-
using System.Collections.Generic;
6-
using System.Threading.Channels;
7-
using System.Threading.Tasks;
84
using Soenneker.Extensions.Task;
95
using Soenneker.Extensions.ValueTask;
106
using Soenneker.Utils.AtomicBool;
117
using Soenneker.Utils.ReusableStringWriter;
8+
using System;
9+
using System.Collections.Generic;
10+
using System.Threading;
11+
using System.Threading.Channels;
12+
using System.Threading.Tasks;
1213
using Xunit;
1314
using Xunit.Sdk;
1415
using Xunit.v3;
1516

1617
namespace Serilog.Sinks.XUnit.Injectable;
1718

18-
/// <inheritdoc cref="IInjectableTestOutputSink"/>
19-
public sealed class InjectableTestOutputSink : IInjectableTestOutputSink, IAsyncDisposable, IDisposable
19+
///<inheritdoc cref="IInjectableTestOutputSink"/>
20+
public sealed class InjectableTestOutputSink : IInjectableTestOutputSink
2021
{
2122
private const string _defaultTemplate = "[{Timestamp:HH:mm:ss} {Level:u3}] {Message:lj}{Exception}";
23+
private const int _backlogCap = 2048; // limit in-memory backlog when helper isn't available
24+
private const int _channelCapacity = 4096; // apply backpressure under heavy logging
25+
26+
private static readonly TimeSpan _drainWait = TimeSpan.FromSeconds(2);
27+
private static readonly TimeSpan _cancelWait = TimeSpan.FromSeconds(3);
2228

2329
private readonly MessageTemplateTextFormatter _fmt;
2430

25-
private readonly Channel<LogEvent> _ch = Channel.CreateUnbounded<LogEvent>(new UnboundedChannelOptions
26-
{ SingleReader = true, SingleWriter = false, AllowSynchronousContinuations = false });
31+
// Bounded channel prevents infinite growth if tests end or helper is missing.
32+
private readonly Channel<LogEvent> _ch = Channel.CreateBounded<LogEvent>(new BoundedChannelOptions(_channelCapacity)
33+
{
34+
SingleReader = true,
35+
SingleWriter = false,
36+
FullMode = BoundedChannelFullMode.DropWrite,
37+
AllowSynchronousContinuations = false
38+
});
2739

40+
private readonly CancellationTokenSource _cts = new();
2841
private readonly Task _readerTask;
2942

3043
private readonly ReusableStringWriter _sw = new();
3144

32-
// Shared with producers/reader – volatile for safe publication
45+
// Volatile so producers/reader see latest references without locks
3346
private volatile ITestOutputHelper? _helper;
3447
private volatile IMessageSink? _sink;
3548

36-
// Only the reader touches this; safe without locks.
49+
// Only the reader loop touches this queue
3750
private readonly Queue<LogEvent> _pending = new();
3851

3952
private readonly AtomicBool _disposed = new();
4053

4154
public InjectableTestOutputSink(string outputTemplate = _defaultTemplate, IFormatProvider? formatProvider = null)
4255
{
4356
_fmt = new MessageTemplateTextFormatter(outputTemplate, formatProvider);
44-
_readerTask = Task.Run(ReadLoop);
57+
_readerTask = Task.Run(() => ReadLoop(_cts.Token));
4558
}
4659

47-
public void Inject(ITestOutputHelper helper, IMessageSink? sink = null)
60+
/// <summary>Inject the current test's output helper (call at test start).</summary>
61+
public void Inject(ITestOutputHelper helper, IMessageSink? diagnosticSink = null)
4862
{
4963
ArgumentNullException.ThrowIfNull(helper);
50-
_helper = helper;
51-
_sink = sink;
64+
_helper = helper; // publish to reader
65+
_sink = diagnosticSink;
5266
}
5367

68+
/// <summary>Serilog pipeline entry point.</summary>
5469
public void Emit(LogEvent logEvent)
5570
{
56-
if (_disposed.IsFalse)
57-
_ch.Writer.TryWrite(logEvent);
71+
if (logEvent is null || _disposed.IsTrue)
72+
return;
73+
74+
_ch.Writer.TryWrite(logEvent); // non-blocking; may drop when full
5875
}
5976

60-
private async Task ReadLoop()
77+
public void Complete()
6178
{
62-
await foreach (LogEvent evt in _ch.Reader.ReadAllAsync()
63-
.ConfigureAwait(false))
64-
{
65-
ITestOutputHelper? helper = _helper; // volatile read
79+
if (_disposed.IsTrue)
80+
return;
6681

67-
if (helper is null)
68-
{
69-
_pending.Enqueue(evt); // buffer until helper arrives
70-
continue;
71-
}
82+
_helper = null; // stop xUnit writes
83+
_ch.Writer.TryComplete(); // prefer graceful drain
84+
// optional: don't cancel here; let Dispose handle fallback cancel on timeout
85+
}
7286

73-
// first, flush any backlog that accumulated pre-inject
74-
while (_pending.Count > 0)
87+
private async Task ReadLoop(CancellationToken ct)
88+
{
89+
try
90+
{
91+
await foreach (LogEvent evt in _ch.Reader.ReadAllAsync(ct)
92+
.ConfigureAwait(false))
7593
{
76-
Write(_pending.Dequeue(), helper);
94+
if (ct.IsCancellationRequested) break;
95+
96+
ITestOutputHelper? helper = _helper; // volatile read
97+
if (helper is null)
98+
{
99+
if (_pending.Count < _backlogCap)
100+
_pending.Enqueue(evt);
101+
continue;
102+
}
103+
104+
// Flush any backlog that accumulated before helper arrived
105+
while (!ct.IsCancellationRequested && _pending.Count > 0)
106+
Write(_pending.Dequeue(), helper);
107+
108+
if (!ct.IsCancellationRequested)
109+
Write(evt, helper);
77110
}
78-
79-
Write(evt, helper);
111+
}
112+
catch (OperationCanceledException)
113+
{
114+
// expected during teardown
115+
}
116+
catch
117+
{
118+
// never let logging crash tests
80119
}
81120
}
82121

83122
private void Write(LogEvent evt, ITestOutputHelper helper)
84123
{
85-
_sw.Reset();
86-
_fmt.Format(evt, _sw);
87-
string message = _sw.Finish();
88-
89-
_sink?.OnMessage(new DiagnosticMessage(message));
90-
91124
try
92125
{
93-
helper.WriteLine(message);
126+
_sw.Reset();
127+
_fmt.Format(evt, _sw);
128+
string message = _sw.Finish();
129+
130+
try
131+
{
132+
_sink?.OnMessage(new DiagnosticMessage(message));
133+
}
134+
catch
135+
{
136+
/* ignore */
137+
}
138+
139+
try
140+
{
141+
helper.WriteLine(message);
142+
}
143+
catch (InvalidOperationException)
144+
{
145+
// test finished; helper invalid
146+
_helper = null;
147+
148+
if (_pending.Count < _backlogCap)
149+
_pending.Enqueue(evt);
150+
}
151+
catch
152+
{
153+
_helper = null;
154+
}
94155
}
95-
catch (InvalidOperationException)
156+
catch
96157
{
97-
// Helper became invalid (test finished) – cache the event
98-
_helper = null;
99-
_pending.Enqueue(evt);
158+
// swallow formatting/writing failures
100159
}
101160
}
102161

103162
public async ValueTask DisposeAsync()
104163
{
105-
if (!_disposed.TrySetTrue())
106-
return;
164+
if (!_disposed.TrySetTrue()) return;
107165

108-
// 1) Tell the reader no more items are coming
109-
_ch.Writer.TryComplete();
166+
_helper = null; // stop xUnit calls after this point
167+
_ch.Writer.TryComplete(); // 1) tell reader: no more items
110168

111-
// 2) Let the reader finish formatting & flushing
112-
await _readerTask.NoSync();
169+
try
170+
{
171+
// 2) give the reader a short window to drain cleanly
172+
await _readerTask.WaitAsync(_drainWait)
173+
.NoSync();
174+
}
175+
catch (TimeoutException)
176+
{
177+
// 3) fallback: force-break the loop if it didn’t finish
178+
await _cts.CancelAsync()
179+
.NoSync();
180+
try
181+
{
182+
await _readerTask.WaitAsync(_cancelWait)
183+
.NoSync();
184+
}
185+
catch
186+
{
187+
/* swallow during teardown */
188+
}
189+
}
190+
catch (OperationCanceledException)
191+
{
192+
/* ok */
193+
}
113194

114-
await _sw.DisposeAsync()
115-
.NoSync();
195+
try
196+
{
197+
await _sw.DisposeAsync()
198+
.NoSync();
199+
}
200+
catch
201+
{
202+
}
203+
204+
_cts.Dispose();
116205
}
117206

118207
public void Dispose()
119208
{
120-
if (!_disposed.TrySetTrue())
121-
return;
209+
if (!_disposed.TrySetTrue()) return;
122210

211+
_helper = null;
123212
_ch.Writer.TryComplete();
124213

125214
try
@@ -129,9 +218,25 @@ public void Dispose()
129218
}
130219
catch
131220
{
132-
// swallow during teardown
221+
_cts.Cancel();
222+
try
223+
{
224+
_readerTask.GetAwaiter()
225+
.GetResult();
226+
}
227+
catch
228+
{
229+
}
230+
}
231+
232+
try
233+
{
234+
_sw.Dispose();
235+
}
236+
catch
237+
{
133238
}
134239

135-
_sw.Dispose();
240+
_cts.Dispose();
136241
}
137242
}

test/Sinks/BlockingCollectionInjectableTestOutputSink.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ public BlockingCollectionInjectableTestOutputSink(string outputTemplate = _defau
3838
private static ReusableStringWriter RentWriter() =>
3939
_threadWriter ??= new ReusableStringWriter();
4040

41+
public void Complete()
42+
{
43+
throw new NotImplementedException();
44+
}
45+
4146
public void Inject(ITestOutputHelper helper, IMessageSink? sink = null)
4247
{
4348
ArgumentNullException.ThrowIfNull(helper);

test/Sinks/ChannelInjectableTestOutputSink.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ public ChannelInjectableTestOutputSink(string outputTemplate = _defaultTemplate,
4343
_readerTask = Task.Run(ReadLoop);
4444
}
4545

46+
public void Complete()
47+
{
48+
throw new NotImplementedException();
49+
}
50+
4651
public void Inject(ITestOutputHelper helper, IMessageSink? sink = null)
4752
{
4853
ArgumentNullException.ThrowIfNull(helper);

test/Sinks/ConcurrentInjectableTestOutputSink.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ public ConcurrentInjectableTestOutputSink(string outputTemplate = _defaultTempla
3131
_fmt = new MessageTemplateTextFormatter(outputTemplate, formatProvider);
3232
}
3333

34+
public void Complete()
35+
{
36+
throw new NotImplementedException();
37+
}
38+
3439
public void Inject(ITestOutputHelper helper, IMessageSink? sink = null)
3540
{
3641
ArgumentNullException.ThrowIfNull(helper);

test/Sinks/OriginalInjectableTestOutputSink.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ public OriginalInjectableTestOutputSink(string outputTemplate = _defaultConsoleO
3535
_textFormatter = new MessageTemplateTextFormatter(outputTemplate, formatProvider);
3636
}
3737

38+
public void Complete()
39+
{
40+
throw new NotImplementedException();
41+
}
42+
3843
public void Inject(ITestOutputHelper testOutputHelper, IMessageSink? messageSink = null)
3944
{
4045
_testOutputHelper = testOutputHelper;

test/Sinks/QueueInjectableTestOutputSink.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ public QueueInjectableTestOutputSink(string outputTemplate = _defaultTemplate, I
3131
_fmt = new MessageTemplateTextFormatter(outputTemplate, formatProvider);
3232
}
3333

34+
public void Complete()
35+
{
36+
throw new NotImplementedException();
37+
}
38+
3439
public void Inject(ITestOutputHelper helper, IMessageSink? sink = null)
3540
{
3641
ArgumentNullException.ThrowIfNull(helper);

0 commit comments

Comments
 (0)