Skip to content

Commit b1659a1

Browse files
authored
fix cancellation deadlock (#18178)
1 parent d4fe1ab commit b1659a1

File tree

1 file changed

+21
-46
lines changed

1 file changed

+21
-46
lines changed

src/Compiler/Facilities/BuildGraph.fs

+21-46
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
module FSharp.Compiler.BuildGraph
44

55
open System.Threading
6-
open System.Threading.Tasks
76
open System.Globalization
87

98
[<RequireQualifiedAccess>]
@@ -40,55 +39,31 @@ type GraphNode<'T> private (computation: Async<'T>, cachedResult: ValueOption<'T
4039
cachedResultNode
4140
else
4241
async {
42+
let! ct = Async.CancellationToken
4343
Interlocked.Increment(&requestCount) |> ignore
44+
let enter = semaphore.WaitAsync(ct)
4445

4546
try
46-
let! ct = Async.CancellationToken
47-
48-
// We must set 'taken' before any implicit cancellation checks
49-
// occur, making sure we are under the protection of the 'try'.
50-
// For example, NodeCode's 'try/finally' (TryFinally) uses async.TryFinally which does
51-
// implicit cancellation checks even before the try is entered, as do the
52-
// de-sugaring of 'do!' and other NodeCode constructs.
53-
let mutable taken = false
54-
55-
try
56-
do!
57-
semaphore
58-
.WaitAsync(ct)
59-
.ContinueWith(
60-
(fun _ -> taken <- true),
61-
(TaskContinuationOptions.NotOnCanceled
62-
||| TaskContinuationOptions.NotOnFaulted
63-
||| TaskContinuationOptions.ExecuteSynchronously)
64-
)
65-
|> Async.AwaitTask
66-
67-
match cachedResult with
68-
| ValueSome value -> return value
69-
| _ ->
70-
let tcs = TaskCompletionSource<'T>()
71-
72-
Async.StartWithContinuations(
73-
async {
74-
Thread.CurrentThread.CurrentUICulture <- GraphNode.culture
75-
return! computation
76-
},
77-
(fun res ->
78-
cachedResult <- ValueSome res
79-
cachedResultNode <- async.Return res
80-
computation <- Unchecked.defaultof<_>
81-
tcs.SetResult(res)),
82-
(fun ex -> tcs.SetException(ex)),
83-
(fun _ -> tcs.SetCanceled()),
84-
ct
85-
)
86-
87-
return! tcs.Task |> Async.AwaitTask
88-
finally
89-
if taken then
90-
semaphore.Release() |> ignore
47+
do! enter |> Async.AwaitTask
48+
49+
match cachedResult with
50+
| ValueSome value -> return value
51+
| _ ->
52+
Thread.CurrentThread.CurrentUICulture <- GraphNode.culture
53+
let! result = computation
54+
cachedResult <- ValueSome result
55+
cachedResultNode <- async.Return result
56+
computation <- Unchecked.defaultof<_>
57+
return result
9158
finally
59+
// At this point, the semaphore awaiter is either already completed or about to get canceled.
60+
// If calling Wait() does not throw an exception it means the semaphore was successfully taken and needs to be released.
61+
try
62+
enter.Wait()
63+
semaphore.Release() |> ignore
64+
with _ ->
65+
()
66+
9267
Interlocked.Decrement(&requestCount) |> ignore
9368
}
9469

0 commit comments

Comments
 (0)