From 616bfd8f75fed4d7ee4f5b00282c69d3100b31e7 Mon Sep 17 00:00:00 2001 From: Jakub Majocha <1760221+majocha@users.noreply.github.com> Date: Tue, 21 May 2024 22:33:01 +0200 Subject: [PATCH 1/4] add test --- .../BuildGraphTests.fs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs b/tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs index e1ba5c1b420..3e45fbbc08d 100644 --- a/tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs +++ b/tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs @@ -374,7 +374,32 @@ module BuildGraphTests = errorCountShouldBe 600 + [] + let ``MultipleDiagnosticsLoggers.Parallel finishes when any computation throws`` () = + + let work i = async { + errorR (ExampleException $"{i}") + } + + let failing = Task.FromException(ExampleException "computation failed") |> Async.AwaitTask + + use _ = UseDiagnosticsLogger (CapturingDiagnosticsLogger "test logger") + + let tasks = [ + failing + for i in 1 .. 300 do work i + ] + + let run = task { + try + do! tasks |> MultipleDiagnosticsLoggers.Parallel |> Async.Ignore + with _ -> () + } + Assert.True( + run.Wait(1000), + "MultipleDiagnosticsLoggers.Parallel did not finish." + ) [] From 001124ffcca58f9c1b4f7083105f3b4d58869f20 Mon Sep 17 00:00:00 2001 From: Jakub Majocha <1760221+majocha@users.noreply.github.com> Date: Wed, 22 May 2024 00:28:54 +0200 Subject: [PATCH 2/4] fix deadlock in replayDiagnostics --- src/Compiler/Facilities/DiagnosticsLogger.fs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Compiler/Facilities/DiagnosticsLogger.fs b/src/Compiler/Facilities/DiagnosticsLogger.fs index bdec9ba40b4..61a0bed5ef4 100644 --- a/src/Compiler/Facilities/DiagnosticsLogger.fs +++ b/src/Compiler/Facilities/DiagnosticsLogger.fs @@ -949,9 +949,19 @@ module MultipleDiagnosticsLoggers = try // We want to restore the current diagnostics context when finished. use _ = new CompilationGlobalsScope() - return! Async.Parallel computationsWithLoggers + let! results = Async.Parallel computationsWithLoggers + do! replayDiagnostics |> Async.AwaitTask + return results finally - replayDiagnostics.Wait() + // When any of the computation throws, Async.Parallel may not start some remaining computations at all. + // We set dummy results for them to allow the task to finish and to not lose any already emitted diagnostics. + if not replayDiagnostics.IsCompleted then + let emptyLogger = CapturingDiagnosticsLogger("empty") + + for tcs in diagnosticsReady do + tcs.TrySetResult(emptyLogger) |> ignore + + replayDiagnostics.Wait() } let Sequential computations = From d1ff7eb61383b9f5db96ed3d61d584758b770b6c Mon Sep 17 00:00:00 2001 From: Jakub Majocha <1760221+majocha@users.noreply.github.com> Date: Wed, 22 May 2024 09:55:51 +0200 Subject: [PATCH 3/4] cleanup --- .../BuildGraphTests.fs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs b/tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs index 3e45fbbc08d..9aeb4a31b13 100644 --- a/tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs +++ b/tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs @@ -376,25 +376,15 @@ module BuildGraphTests = [] let ``MultipleDiagnosticsLoggers.Parallel finishes when any computation throws`` () = - - let work i = async { - errorR (ExampleException $"{i}") - } - - let failing = Task.FromException(ExampleException "computation failed") |> Async.AwaitTask - use _ = UseDiagnosticsLogger (CapturingDiagnosticsLogger "test logger") let tasks = [ - failing - for i in 1 .. 300 do work i + async { failwith "computation failed" } + for i in 1 .. 300 do async { errorR (ExampleException $"{i}") } ] - let run = task { - try - do! tasks |> MultipleDiagnosticsLoggers.Parallel |> Async.Ignore - with _ -> () - } + let run = + tasks |> MultipleDiagnosticsLoggers.Parallel |> Async.Catch |> Async.StartAsTask Assert.True( run.Wait(1000), From 5e3aa17f5952ff39e09cb0b21f0601aca7959f27 Mon Sep 17 00:00:00 2001 From: Jakub Majocha <1760221+majocha@users.noreply.github.com> Date: Wed, 22 May 2024 12:23:45 +0200 Subject: [PATCH 4/4] test started tasks --- tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs b/tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs index 9aeb4a31b13..83238e4b7fc 100644 --- a/tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs +++ b/tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs @@ -376,11 +376,18 @@ module BuildGraphTests = [] let ``MultipleDiagnosticsLoggers.Parallel finishes when any computation throws`` () = + + let mutable count = 0 use _ = UseDiagnosticsLogger (CapturingDiagnosticsLogger "test logger") let tasks = [ async { failwith "computation failed" } - for i in 1 .. 300 do async { errorR (ExampleException $"{i}") } + + for i in 1 .. 300 do + async { + Interlocked.Increment(&count) |> ignore + errorR (ExampleException $"{i}") + } ] let run = @@ -391,6 +398,9 @@ module BuildGraphTests = "MultipleDiagnosticsLoggers.Parallel did not finish." ) + // Diagnostics from all started tasks should be collected despite the exception. + errorCountShouldBe count + [] let ``AsyncLocal diagnostics context flows correctly`` () =