dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.83k stars 773 forks source link

`MultipleDiagnosticsLoggers.Parallel` deadlock #17205

Closed ForNeVeR closed 1 month ago

ForNeVeR commented 1 month ago

We observe a deadlock in FCS, likely caused by incorrect diagnostic behavior in MultipleDiagnosticsLoggers.Parallel

Consider the code: https://github.com/dotnet/fsharp/blob/cdd67c76624f07ce14f798f93b6f6512fa6e0604/src/Compiler/Facilities/DiagnosticsLogger.fs#L916-L955

Here, it tries to replicate the same behavior as Async.Parallel, but with additional behavior of reporting some statistics to compiler diagnostic engine.

To do that, essentially it works like this:

  1. Allocates N TaskCompletionSource objects
  2. Connects every input Async with a corresponding TaskCompletionSource
  3. Starts the input computations using Async.Parallel
  4. In the finally block, waits for replayDiagnostics — a task that collects every allocated TaskCompletionSource and reports the results

There's a problem, though: Async.Parallel doesn't give guarantees that it will start every input computation!

It has a short-circuiting property and will stop spawning new computations if it sees an error. Example code to see this behavior:

let failedTask = Task.FromException(Exception("tatata"))
let asyncs = [
    Async.AwaitTask failedTask

    for x in 1..300 do
        async {
            printfn $"{x}"
            failwith "error"
        }
]

let a = Async.Parallel asyncs
async {
    let! results = a
    return ()
} |> Async.RunSynchronously

You will see that it never executes all print 1…print 300 — because of this short-circuiting behavior of Async.Parallel.

If this property of Async.Parallel occurs in the compiler code (i.e. a computation throws an exception), then certain TaskCompletionSource objects will be orphaned (never completed, since their computations were never started to begin with), and thus MultipleDiagnosticsLoggers.Parallel call will never complete.

majocha commented 1 month ago

There's a problem, though: Async.Parallel doesn't give guarantees that it will start every input computation!

@ForNeVeR, thanks for the detailed analysis. I'll try to come up with some fix for this.