Should support concurrency better

Some significant allocation/performance improvements

Author: soenneker

sample/Startup.cs

@@ -1,6 +1,5 @@
 using Microsoft.AspNetCore.Builder;
 using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.Logging;
 using SampleApi.Utils;
 
 namespace SampleApi;
@@ -21,7 +20,7 @@ public void ConfigureServices(IServiceCollection services)
     }
 
     // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
-    public void Configure(IApplicationBuilder app, ILogger<Startup> logger)
+    public void Configure(IApplicationBuilder app)
     {
         app.UseRouting();
 

src/Serilog.Sinks.XUnit.Injectable/Extensions/InjectableTestOutputExtension.cs

@@ -31,11 +31,9 @@ public static class InjectableTestOutputExtension
     public static LoggerConfiguration InjectableTestOutput(this LoggerSinkConfiguration sinkConfiguration, IInjectableTestOutputSink sink,
         LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum, LoggingLevelSwitch? levelSwitch = null)
     {
-        if (sinkConfiguration == null)
-            throw new ArgumentNullException(nameof(sinkConfiguration));
+        ArgumentNullException.ThrowIfNull(sinkConfiguration, nameof(sinkConfiguration));
 
-        if (sink == null)
-            throw new ArgumentNullException(nameof(sink));
+        ArgumentNullException.ThrowIfNull(sink, nameof(sink));
 
         return sinkConfiguration.Sink(sink, restrictedToMinimumLevel, levelSwitch);
     }

src/Serilog.Sinks.XUnit.Injectable/InjectableTestOutputSink.cs

@@ -1,6 +1,7 @@
 using System;
-using System.Collections.Generic;
-using System.IO;
+using System.Collections.Concurrent;
+using System.Text;
+using System.Threading;
 using Serilog.Events;
 using Serilog.Formatting.Display;
 using Serilog.Sinks.XUnit.Injectable.Abstract;
@@ -10,79 +11,87 @@
 
 namespace Serilog.Sinks.XUnit.Injectable;
 
-/// <inheritdoc cref="IInjectableTestOutputSink"/>
+///<inheritdoc cref="IInjectableTestOutputSink"/>
 public sealed class InjectableTestOutputSink : IInjectableTestOutputSink
 {
-    private readonly Stack<LogEvent> _cachedLogEvents;
-    private readonly MessageTemplateTextFormatter _textFormatter;
-    private IMessageSink? _messageSink;
-    private ITestOutputHelper? _testOutputHelper;
-
-    private const string _defaultConsoleOutputTemplate = "[{Timestamp:HH:mm:ss} {Level:u3}] {Message:lj}{NewLine}{Exception}";
-
-    /// <summary>
-    ///     Use this ctor for injecting into the DI container
-    /// </summary>
-    /// <param name="outputTemplate">A message template describing the format used to write to the sink.
-    /// the default is <code>"[{Timestamp:HH:mm:ss} {Level:u3}] {Message:lj}{NewLine}{Exception}"</code>.</param>
-    /// <param name="formatProvider">Supplies culture-specific formatting information, or null.</param>
-    /// <returns>Configuration object allowing method chaining.</returns>
-    public InjectableTestOutputSink(string outputTemplate = _defaultConsoleOutputTemplate, IFormatProvider? formatProvider = null)
-    {
-        _cachedLogEvents = new Stack<LogEvent>();
+    private readonly Lock _lock = new(); // guards flush+write
+    private readonly ConcurrentQueue<LogEvent> _cache = new();
+    private readonly MessageTemplateTextFormatter _fmt;
+
+    private volatile ITestOutputHelper? _helper; // set once, then read
+    private volatile IMessageSink? _sink;
+
+    private const string _defaultTemplate = "[{Timestamp:HH:mm:ss} {Level:u3}] {Message:lj}{Exception}";
+
+    [ThreadStatic] private static StringBuilder? _sb;
 
-        _textFormatter = new MessageTemplateTextFormatter(outputTemplate, formatProvider);
+    [ThreadStatic] private static ReusableStringWriter? _sw;
+
+    public InjectableTestOutputSink(string outputTemplate = _defaultTemplate, IFormatProvider? formatProvider = null)
+    {
+        _fmt = new MessageTemplateTextFormatter(outputTemplate, formatProvider);
     }
 
-    public void Inject(ITestOutputHelper testOutputHelper, IMessageSink? messageSink = null)
+    public void Inject(ITestOutputHelper helper, IMessageSink? sink = null)
     {
-        _testOutputHelper = testOutputHelper;
-        _messageSink = messageSink;
+        ArgumentNullException.ThrowIfNull(helper);
+
+        // one short, guaranteed exit scope
+        using Lock.Scope _ = _lock.EnterScope();
+
+        _helper = helper;
+        _sink = sink;
+        FlushLocked();
     }
 
     public void Emit(LogEvent logEvent)
     {
-        if (_testOutputHelper == null)
+        ArgumentNullException.ThrowIfNull(logEvent);
+
+        // FAST-PATH: helper not yet available – enqueue without locking
+        ITestOutputHelper? helperSnapshot = _helper; // direct volatile read
+
+        if (helperSnapshot is null)
         {
-            _cachedLogEvents.Push(logEvent);
+            _cache.Enqueue(logEvent);
             return;
         }
 
-        FlushCachedLogEvents();
-        Write(logEvent);
+
+        using Lock.Scope _ = _lock.EnterScope();
+
+        // Flush anything another thread buffered before we obtained the scope
+        FlushLocked();
+        WriteLocked(logEvent);
     }
 
-    private void FlushCachedLogEvents()
+    private void FlushLocked()
     {
-        while (_cachedLogEvents.Count > 0)
-        {
-            Write(_cachedLogEvents.Pop());
-        }
+        while (_cache.TryDequeue(out LogEvent? queued))
+            WriteLocked(queued);
     }
 
-    /// <summary>
-    ///     Emits the provided log event from a sink
-    /// </summary>
-    /// <param name="logEvent">The event being logged</param>
-    private void Write(LogEvent logEvent)
+    private void WriteLocked(LogEvent evt)
     {
-        if (logEvent == null)
-            throw new ArgumentNullException(nameof(logEvent));
+        // rent / reset thread-local buffers
+        StringBuilder sb = _sb ??= new StringBuilder(256);
+        sb.Clear();
 
-        var renderSpace = new StringWriter();
-        _textFormatter.Format(logEvent, renderSpace);
+        ReusableStringWriter sw = _sw ??= new ReusableStringWriter(sb);
+        sw.Reset();
 
-        string message = renderSpace.ToString().Trim();
+        _fmt.Format(evt, sw);
+        var message = sb.ToString(); // single unavoidable alloc
 
-        _messageSink?.OnMessage(new DiagnosticMessage(message));
+        _sink?.OnMessage(new DiagnosticMessage(message));
 
         try
         {
-            _testOutputHelper?.WriteLine(message);
+            _helper?.WriteLine(message);
         }
-        catch (InvalidOperationException ex)
+        catch (InvalidOperationException)
         {
-            // Typically no test is active
+            /* test already finished – swallow */
         }
     }
 }

src/Serilog.Sinks.XUnit.Injectable/ReusableStringWriter.cs

@@ -0,0 +1,13 @@
+using System.IO;
+using System.Text;
+
+namespace Serilog.Sinks.XUnit.Injectable;
+
+internal sealed class ReusableStringWriter : StringWriter
+{
+    public ReusableStringWriter(StringBuilder sb) : base(sb)
+    {
+    }
+
+    public void Reset() => GetStringBuilder().Clear();
+}

src/Serilog.Sinks.XUnit.Injectable/Serilog.Sinks.XUnit.Injectable.csproj

@@ -1,7 +1,9 @@
-<?xml version="1.0" encoding="utf-8"?><Project Sdk="Microsoft.NET.Sdk">
+<?xml version="1.0" encoding="utf-8"?>
+
+<Project Sdk="Microsoft.NET.Sdk">
 
     <PropertyGroup>
-        <TargetFrameworks>net8.0;net9.0</TargetFrameworks>
+        <TargetFrameworks>net9.0</TargetFrameworks>
         <Nullable>enable</Nullable>
         <NoWarn>$(NoWarn);1591</NoWarn>
         <Platforms>AnyCPU</Platforms>
@@ -30,7 +32,8 @@
         <LangVersion>latest</LangVersion>
         <PackageReadmeFile>README.md</PackageReadmeFile>
         <PackageIcon>icon.png</PackageIcon>
-    <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild></PropertyGroup>
+        <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
+    </PropertyGroup>
     <ItemGroup>
         <None Include="..\..\README.md" Pack="true" PackagePath="\" />
         <None Include="..\..\icon.png" Pack="true" PackagePath="\" />

test/Integration/ApiFixture.cs

@@ -11,7 +11,7 @@ namespace Serilog.Sinks.XUnit.Injectable.Tests.Integration;
 
 public class ApiFixture : IAsyncLifetime
 {
-    public WebApplicationFactory<Program> ApiFactory { get; set; } = default!;
+    public WebApplicationFactory<Program> ApiFactory { get; set; } = null!;
 
     public ValueTask InitializeAsync()
     {

test/Integration/ApiTests.cs

@@ -24,4 +24,14 @@ public async Task Get_should_have_log_messages_and_be_successful()
         HttpResponseMessage response = await _client.GetAsync("/", TestContext.Current.CancellationToken);
         response.EnsureSuccessStatusCode();
     }
+
+    [Fact]
+    public async Task Get_concurrent()
+    {
+        Task<HttpResponseMessage> task1= _client.GetAsync("/", TestContext.Current.CancellationToken);
+        Task<HttpResponseMessage> task2 = _client.GetAsync("/", TestContext.Current.CancellationToken);
+        Task<HttpResponseMessage> task3 = _client.GetAsync("/", TestContext.Current.CancellationToken);
+
+        await Task.WhenAll(task1, task2, task3);
+    }
 }

test/Unit/UnitFixture.cs

@@ -10,7 +10,7 @@ namespace Serilog.Sinks.XUnit.Injectable.Tests.Unit;
 
 public class UnitFixture : IAsyncLifetime
 {
-    public ServiceProvider ServiceProvider { get; set; } = default!;
+    public ServiceProvider ServiceProvider { get; set; } = null!;
 
     protected IServiceCollection Services { get; set; }