Skip to content

Commit 6420e36

Browse files
Fix MultipleDiagnosticsLoggers.Parallel deadlock (#17208)
* add test * fix deadlock in replayDiagnostics * cleanup * test started tasks --------- Co-authored-by: Vlad Zarytovskii <[email protected]>
1 parent 0d535b1 commit 6420e36

File tree

2 files changed

+37
-2
lines changed

2 files changed

+37
-2
lines changed

src/Compiler/Facilities/DiagnosticsLogger.fs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -949,9 +949,19 @@ module MultipleDiagnosticsLoggers =
949949
try
950950
// We want to restore the current diagnostics context when finished.
951951
use _ = new CompilationGlobalsScope()
952-
return! Async.Parallel computationsWithLoggers
952+
let! results = Async.Parallel computationsWithLoggers
953+
do! replayDiagnostics |> Async.AwaitTask
954+
return results
953955
finally
954-
replayDiagnostics.Wait()
956+
// When any of the computation throws, Async.Parallel may not start some remaining computations at all.
957+
// We set dummy results for them to allow the task to finish and to not lose any already emitted diagnostics.
958+
if not replayDiagnostics.IsCompleted then
959+
let emptyLogger = CapturingDiagnosticsLogger("empty")
960+
961+
for tcs in diagnosticsReady do
962+
tcs.TrySetResult(emptyLogger) |> ignore
963+
964+
replayDiagnostics.Wait()
955965
}
956966

957967
let Sequential computations =

tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,32 @@ module BuildGraphTests =
374374

375375
errorCountShouldBe 600
376376

377+
[<Fact>]
378+
let ``MultipleDiagnosticsLoggers.Parallel finishes when any computation throws`` () =
379+
380+
let mutable count = 0
381+
use _ = UseDiagnosticsLogger (CapturingDiagnosticsLogger "test logger")
382+
383+
let tasks = [
384+
async { failwith "computation failed" }
385+
386+
for i in 1 .. 300 do
387+
async {
388+
Interlocked.Increment(&count) |> ignore
389+
errorR (ExampleException $"{i}")
390+
}
391+
]
392+
393+
let run =
394+
tasks |> MultipleDiagnosticsLoggers.Parallel |> Async.Catch |> Async.StartAsTask
395+
396+
Assert.True(
397+
run.Wait(1000),
398+
"MultipleDiagnosticsLoggers.Parallel did not finish."
399+
)
377400

401+
// Diagnostics from all started tasks should be collected despite the exception.
402+
errorCountShouldBe count
378403

379404

380405
[<Fact>]

0 commit comments

Comments
 (0)