Fix thread safety when OpenAPI document is downloaded in parallel

Author: bkoelman

src/JsonApiDotNetCore.OpenApi.Swashbuckle/JsonApiActionDescriptorCollectionProvider.cs

@@ -1,3 +1,4 @@
+using System.Collections.Concurrent;
 using System.Net;
 using System.Reflection;
 using JsonApiDotNetCore.Configuration;
@@ -36,8 +37,10 @@ internal sealed partial class JsonApiActionDescriptorCollectionProvider : IActio
     private readonly JsonApiEndpointMetadataProvider _jsonApiEndpointMetadataProvider;
     private readonly IJsonApiOptions _options;
     private readonly ILogger<JsonApiActionDescriptorCollectionProvider> _logger;
+    private readonly ConcurrentDictionary<int, Lazy<ActionDescriptorCollection>> _versionedActionDescriptorCache = new();
 
-    public ActionDescriptorCollection ActionDescriptors => GetActionDescriptors();
+    public ActionDescriptorCollection ActionDescriptors =>
+        _versionedActionDescriptorCache.GetOrAdd(_defaultProvider.ActionDescriptors.Version, LazyGetActionDescriptors).Value;
 
     public JsonApiActionDescriptorCollectionProvider(IActionDescriptorCollectionProvider defaultProvider, IControllerResourceMapping controllerResourceMapping,
         JsonApiEndpointMetadataProvider jsonApiEndpointMetadataProvider, IJsonApiOptions options, ILogger<JsonApiActionDescriptorCollectionProvider> logger)
@@ -55,7 +58,13 @@ public JsonApiActionDescriptorCollectionProvider(IActionDescriptorCollectionProv
         _logger = logger;
     }
 
-    private ActionDescriptorCollection GetActionDescriptors()
+    private Lazy<ActionDescriptorCollection> LazyGetActionDescriptors(int version)
+    {
+        // https://andrewlock.net/making-getoradd-on-concurrentdictionary-thread-safe-using-lazy/
+        return new Lazy<ActionDescriptorCollection>(() => GetActionDescriptors(version), LazyThreadSafetyMode.ExecutionAndPublication);
+    }
+
+    private ActionDescriptorCollection GetActionDescriptors(int version)
     {
         List<ActionDescriptor> descriptors = [];
 
@@ -106,8 +115,7 @@ private ActionDescriptorCollection GetActionDescriptors()
             descriptors.Add(descriptor);
         }
 
-        int descriptorVersion = _defaultProvider.ActionDescriptors.Version;
-        return new ActionDescriptorCollection(descriptors.AsReadOnly(), descriptorVersion);
+        return new ActionDescriptorCollection(descriptors.AsReadOnly(), version);
     }
 
     internal static bool IsVisibleEndpoint(ActionDescriptor descriptor)

src/JsonApiDotNetCore.OpenApi.Swashbuckle/SchemaGenerationTracer.cs

@@ -1,3 +1,4 @@
+using System.Runtime.CompilerServices;
 using JsonApiDotNetCore.Resources.Annotations;
 using JsonApiDotNetCore.Serialization.Objects;
 using Microsoft.Extensions.Logging;
@@ -87,7 +88,7 @@ private static string GetSchemaTypeName(Type type)
 
     private sealed partial class SchemaGenerationTraceScope : ISchemaGenerationTraceScope
     {
-        private static readonly AsyncLocal<int> RecursionDepthAsyncLocal = new();
+        private static readonly AsyncLocal<StrongBox<int>> RecursionDepthAsyncLocal = new();
 
         private readonly ILogger _logger;
         private readonly string _schemaTypeName;
@@ -101,8 +102,10 @@ public SchemaGenerationTraceScope(ILogger logger, string schemaTypeName)
             _logger = logger;
             _schemaTypeName = schemaTypeName;
 
-            RecursionDepthAsyncLocal.Value++;
-            LogStarted(RecursionDepthAsyncLocal.Value, _schemaTypeName);
+            RecursionDepthAsyncLocal.Value ??= new StrongBox<int>(0);
+            int depth = Interlocked.Increment(ref RecursionDepthAsyncLocal.Value.Value);
+
+            LogStarted(depth, _schemaTypeName);
         }
 
         public void TraceSucceeded(string schemaId)
@@ -112,16 +115,18 @@ public void TraceSucceeded(string schemaId)
 
         public void Dispose()
         {
+            int depth = RecursionDepthAsyncLocal.Value!.Value;
+
             if (_schemaId != null)
             {
-                LogSucceeded(RecursionDepthAsyncLocal.Value, _schemaTypeName, _schemaId);
+                LogSucceeded(depth, _schemaTypeName, _schemaId);
             }
             else
             {
-                LogFailed(RecursionDepthAsyncLocal.Value, _schemaTypeName);
+                LogFailed(depth, _schemaTypeName);
             }
 
-            RecursionDepthAsyncLocal.Value--;
+            Interlocked.Decrement(ref RecursionDepthAsyncLocal.Value.Value);
         }
 
         [LoggerMessage(Level = LogLevel.Trace, SkipEnabledCheck = true, Message = "({Depth:D2}) Started for {SchemaTypeName}.")]

test/OpenApiTests/OpenApiTestContext.cs

@@ -28,7 +28,7 @@ internal async Task<JsonElement> GetSwaggerDocumentAsync()
         return await _lazySwaggerDocument.Value;
     }
 
-    private async Task<JsonElement> CreateSwaggerDocumentAsync()
+    internal async Task<JsonElement> CreateSwaggerDocumentAsync()
     {
         string content = await GetAsync("/swagger/v1/swagger.json");
 

test/OpenApiTests/ResourceInheritance/ConcurrencyTests.cs

@@ -0,0 +1,40 @@
+using FluentAssertions;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace OpenApiTests.ResourceInheritance;
+
+public sealed class ConcurrencyTests : ResourceInheritanceTests
+{
+    private readonly OpenApiTestContext<OpenApiStartup<ResourceInheritanceDbContext>, ResourceInheritanceDbContext> _testContext;
+
+    public ConcurrencyTests(OpenApiTestContext<OpenApiStartup<ResourceInheritanceDbContext>, ResourceInheritanceDbContext> testContext,
+        ITestOutputHelper testOutputHelper)
+        : base(testContext, testOutputHelper, true, false)
+    {
+        _testContext = testContext;
+
+        testContext.ConfigureServices(services => services.AddLogging(loggingBuilder => loggingBuilder.ClearProviders()));
+    }
+
+    [Fact]
+    public async Task Can_download_OpenAPI_documents_in_parallel()
+    {
+        // Arrange
+        const int count = 15;
+        var downloadTasks = new Task[count];
+
+        for (int index = 0; index < count; index++)
+        {
+            downloadTasks[index] = _testContext.CreateSwaggerDocumentAsync();
+        }
+
+        // Act
+        Func<Task> action = async () => await Task.WhenAll(downloadTasks);
+
+        // Assert
+        await action.Should().NotThrowAsync();
+    }
+}