fix(inbound): single-flight method compile so concurrent first callers share one assembly (review 2026-10-21 05/M1)

src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs

@@ -40,6 +40,9 @@ public class InboundScriptExecutor
     // not safe for concurrent read/write, so a ConcurrentDictionary is used throughout.
     private readonly ConcurrentDictionary<string, CachedHandler> _scriptHandlers = new();
 
+    // In-flight compiles keyed by method name + script text (see CompileSingleFlight).
+    private readonly ConcurrentDictionary<string, Lazy<(Func<InboundScriptContext, Task<object?>>? Handler, IReadOnlyList<string> Errors)>> _inFlightCompiles = new();
+
     // A script that fails to compile (or violates the trust model)
     // is recorded here — keyed by method name, valued by the exact script text
     // that failed — so it is compiled at most once. Without this, every subsequent
@@ -188,7 +191,7 @@ public class InboundScriptExecutor
     /// <returns>True if successfully compiled and registered, false otherwise.</returns>
     public bool CompileAndRegister(ApiMethod method, out IReadOnlyList<string> errors)
     {
-        var (handler, compileErrors) = Compile(method);
+        var (handler, compileErrors) = CompileSingleFlight(method);
         if (handler == null)
         {
             // Record the failure so the lazy-compile path does not
@@ -207,6 +210,28 @@ public class InboundScriptExecutor
         return Register(method, handler);
     }
 
+    /// <summary>
+    /// Single-flight wrapper over <see cref="Compile"/>, keyed by method name + script
+    /// revision: N concurrent first-callers of a new/changed method share ONE compile (and
+    /// one non-collectible loaded assembly) instead of each running Roslyn. The entry is
+    /// removed as soon as the compile finishes, so a later edit recompiles normally.
+    /// </summary>
+    private (Func<InboundScriptContext, Task<object?>>? Handler, IReadOnlyList<string> Errors) CompileSingleFlight(ApiMethod method)
+    {
+        var key = method.Name + "\0" + (method.Script ?? string.Empty);
+        var lazy = _inFlightCompiles.GetOrAdd(key, _ =>
+            new Lazy<(Func<InboundScriptContext, Task<object?>>?, IReadOnlyList<string>)>(
+                () => Compile(method), LazyThreadSafetyMode.ExecutionAndPublication));
+        try
+        {
+            return lazy.Value;
+        }
+        finally
+        {
+            _inFlightCompiles.TryRemove(new KeyValuePair<string, Lazy<(Func<InboundScriptContext, Task<object?>>?, IReadOnlyList<string>)>>(key, lazy));
+        }
+    }
+
     private bool Register(ApiMethod method, Func<InboundScriptContext, Task<object?>> handler)
     {
         _scriptHandlers[method.Name] = new CachedHandler(method.Script, handler);
@@ -518,7 +543,7 @@ public class InboundScriptExecutor
                 // Lazy compile (handles methods created after startup, and stale/known-bad
                 // recompiles). Compile outside the cache so a failed compile is not stored,
                 // then add so concurrent first-callers share a single handler instance.
-                var (compiled, _) = Compile(method);
+                var (compiled, _) = CompileSingleFlight(method);
                 if (compiled == null)
                 {
                     // Cache the failure so the next request short-circuits above.

tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundScriptExecutorTests.cs

@@ -514,6 +514,37 @@ public class InboundScriptExecutorTests
         Assert.Empty(errors);
     }
 
+    /// <summary>Verifies that concurrent first callers of a new method share a single compile.</summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task ExecuteAsync_ConcurrentFirstCallers_ShareOneCompile()
+    {
+        // Review 2026-10-21 05/M1: N concurrent first-callers used to each run Roslyn (N
+        // non-collectible loaded assemblies for one revision). Single-flight per name+revision.
+        var logger = new CompileCountingLogger();
+        var executor = new InboundScriptExecutor(logger, Substitute.For<IServiceProvider>());
+        var method = new ApiMethod("burst", "var x = 0; for (int i = 0; i < 10; i++) x += i; return x;") { Id = 1, TimeoutSeconds = 30 };
+
+        var results = await Task.WhenAll(Enumerable.Range(0, 12).Select(_ => Task.Run(() =>
+            executor.ExecuteAsync(method, new Dictionary<string, object?>(), _route, TimeSpan.FromSeconds(30)))));
+
+        Assert.All(results, r => Assert.True(r.Success, r.ErrorMessage));
+        Assert.Equal(1, logger.CompileCount);
+    }
+
+    private sealed class CompileCountingLogger : ILogger<InboundScriptExecutor>
+    {
+        private int _count;
+        public int CompileCount => _count;
+        public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
+        public bool IsEnabled(LogLevel logLevel) => true;
+        public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
+        {
+            if (formatter(state, exception).EndsWith("script compiled", StringComparison.Ordinal))
+                Interlocked.Increment(ref _count);
+        }
+    }
+
     /// <summary>Verifies that executing after a non-compiling update surfaces the compile failure.</summary>
     /// <returns>A task that represents the asynchronous operation.</returns>
     [Fact]