From 792578bce5faf0356328c23bc74c8282734882b2 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Tue, 15 Sep 2026 02:21:23 +0200 Subject: [PATCH 1/2] Make CapturingDiagnosticsLogger safe for concurrent writers Graph type-checking in the transparent compiler starts each node with Async.Start, so every node inherits the parent AsyncMemoize job's "cache" logger and commits its own delayed diagnostics into it from its own thread. Racing ResizeArray.Add calls can leave Count above the backing array's length, and every later ToArray() in CommitDelayedDiagnostics then throws ArgumentException ("Source array was not long enough"), which poisons the cached job. Add under a lock, and snapshot under the same lock for Diagnostics and CommitDelayedDiagnostics; the replay stays outside the lock. Co-Authored-By: Claude Opus 5 (1M context) --- src/Compiler/Facilities/DiagnosticsLogger.fs | 11 ++++++----- .../BuildGraphTests.fs | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/Compiler/Facilities/DiagnosticsLogger.fs b/src/Compiler/Facilities/DiagnosticsLogger.fs index 7cc44d88366..7b86807006d 100644 --- a/src/Compiler/Facilities/DiagnosticsLogger.fs +++ b/src/Compiler/Facilities/DiagnosticsLogger.fs @@ -399,18 +399,19 @@ type CapturingDiagnosticsLogger(nm, ?eagerFormat) = | None -> diagnostic | Some f -> f diagnostic - if diagnostic.Severity = FSharpDiagnosticSeverity.Error then - errorCount <- errorCount + 1 + lock diagnostics (fun () -> + if diagnostic.Severity = FSharpDiagnosticSeverity.Error then + errorCount <- errorCount + 1 - diagnostics.Add(diagnostic) + diagnostics.Add(diagnostic)) override _.ErrorCount = errorCount - member _.Diagnostics = diagnostics |> Seq.toList + member _.Diagnostics = lock diagnostics (fun () -> List.ofSeq diagnostics) member _.CommitDelayedDiagnostics(diagnosticsLogger: DiagnosticsLogger) = // Eagerly grab all the errors and warnings from the mutable collection - let errors = diagnostics.ToArray() + let errors = lock diagnostics diagnostics.ToArray errors |> Array.iter diagnosticsLogger.DiagnosticSink let buildPhase = AsyncLocal() diff --git a/tests/FSharp.Compiler.Service.Tests/BuildGraphTests.fs b/tests/FSharp.Compiler.Service.Tests/BuildGraphTests.fs index 6bd2a15585b..d801f7bcfd4 100644 --- a/tests/FSharp.Compiler.Service.Tests/BuildGraphTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/BuildGraphTests.fs @@ -338,6 +338,22 @@ module BuildGraphTests = Parallel.Invoke(task1, task2) + [] + let ``CapturingDiagnosticsLogger keeps every diagnostic reported concurrently`` () = + let count = 100_000 + let logger = CapturingDiagnosticsLogger "concurrent writers" + + do + use _ = UseDiagnosticsLogger logger + Parallel.For(0, count, fun _ -> errorR TestException) |> ignore + + logger.ErrorCount |> Assert.shouldBe count + logger.Diagnostics.Length |> Assert.shouldBe count + + let target = CapturingDiagnosticsLogger "commit target" + logger.CommitDelayedDiagnostics target + target.ErrorCount |> Assert.shouldBe count + type internal DiagnosticsLoggerWithCallback(callback) = inherit CapturingDiagnosticsLogger("test") From 4e322c86ab3c1740d635846f1cb28450aad010c6 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Tue, 15 Sep 2026 02:22:08 +0200 Subject: [PATCH 2/2] Add release notes for CapturingDiagnosticsLogger thread safety Co-Authored-By: Claude Opus 5 (1M context) --- docs/release-notes/.FSharp.Compiler.Service/11.0.100.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 4df01e7ceb0..c61888ab103 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -2,6 +2,7 @@ * Fix internal error FS0192 "Iterate2D" when a `[]` parameter auto-quotes an argument that captures a not-yet-generalized use of an inferred generically-recursive function. The auto-quoted (`Expr.WithValue`) copy now keeps a fresh link to the recursive-value use so it receives the same inferred type arguments as the executable expression at the letrec point. ([Issue #20379](https://github.com/dotnet/fsharp/issues/20379)) * Fix `NativePtr.stackalloc` nested in a larger expression (e.g. a call argument or the right of an assignment) producing an assembly that throws `InvalidProgramException` at load. ([Issue #8083](https://github.com/dotnet/fsharp/issues/8083), [PR #20302](https://github.com/dotnet/fsharp/pull/20302)) +* Fix `CapturingDiagnosticsLogger` losing diagnostics or throwing `ArgumentException` ("Source array was not long enough") on every later request for a cached transparent-compiler result, after parallel graph type-checking nodes wrote into it concurrently. ([PR #20553](https://github.com/dotnet/fsharp/pull/20553)) * Fix internal error "Unexpected generalized type variables when compiling an active pattern" when an active pattern is used in a `let` binding whose right-hand side is a generic value, e.g. `let (T) = id`. Such a binding is now checked like the equivalent `match` and is not generalized. ([Issue #16856](https://github.com/dotnet/fsharp/issues/16856), [PR #20383](https://github.com/dotnet/fsharp/pull/20383)) * Fix Release-only (`--optimize+`) `System.InvalidProgramException` from `Seq.collect` / `yield!` over a value-type (struct) collection implementing `seq<'T>` (e.g. `ImmutableArray<_>`) when materialised with `List.ofSeq` / `Seq.toList` / `Seq.toArray` or a list/array comprehension. The collector lowering now boxes a struct sub-collection to `seq<'T>` before calling `AddMany`/`AddManyAndClose` (matching the coercion the type checker already inserts for `yield!`), and uses `unit` as the try/finally result type instead of the body type (removing a spurious `ldnull` store). ([Issue #20203](https://github.com/dotnet/fsharp/issues/20203)) * Fix recursive inline SRTP resolution being truncated by one currying level (e.g. FSharpPlus `memoizeN`), a regression from the function-domain unification order change in [PR #15181](https://github.com/dotnet/fsharp/pull/15181); the contravariant domain now keeps the inference variable that still carries the pending member constraint. ([PR #20247](https://github.com/dotnet/fsharp/pull/20247))