TryFinallyAsync implementation ignores potential exceptions in TryFinally
- Dominant language
- F#
- Stars
- 4.3k
- Forks
- 876
- Avg merge
- 4d 22h
- Merged PRs (30d)
- 144
Description
Currently implementation of TryFinallyAsync is defined as:
```fsharp
member inline internal this.TryFinallyAsync
(
body: TaskCode<'TOverall, 'T>,
compensation: unit -> ValueTask
) : TaskCode<'TOverall, 'T> =
ResumableCode.TryFinallyAsync(
body,
ResumableCode<_, _>(fun sm ->
if __useResumableCode then
let mutable __stack_condition_fin = true
let __stack_vtask = compensation ()
if not __stack_vtask.IsCompleted then
let mutable awaiter = __stack_vtask.GetAwaiter()
let __stack_yield_fin = ResumableCode.Yield().Invoke(&sm)
__stack_condition_fin <- __stack_yield_fin
if not __stack_condition_fin then
sm.Data.MethodBuilder.AwaitUnsafeOnCompleted(&awaiter, &sm)
__stack_condition_fin
else
let vtask = compensation ()
let mutable awaiter = vtask.GetAwaiter()
let cont =
TaskResumptionFunc<'TOverall>(fun sm ->
awaiter.GetResult() |> ignore
true)
// shortcut to continue immediately
if awaiter.IsCompleted then
true
else
sm.ResumptionDynamicInfo.ResumptionData <- (awaiter :> ICriticalNotifyCompletion)
sm.ResumptionDynamicInfo.ResumptionFunc <- cont
false)
)
```
Note that it never calls `awaiter.GetResult()` meaning that if ValueTask was compelted synchronously it will throw but if not then it will just ignore result of the async computation.
Proposed fix is (matches `.Bind` implementation for example):
```fsharp
member inline internal this.TryFinallyAsync
(
body: TaskCode<'TOverall, 'T>,
compensation: unit -> ValueTask
) : TaskCode<'TOverall, 'T> =
ResumableCode.TryFinallyAsync(
body,
ResumableCode<_, _>(fun sm ->
if __useResumableCode then
let mutable __stack_condition_fin = true
let __stack_vtask = compensation ()
let mutable awaiter = __stack_vtask.GetAwaiter()
if not awaiter.IsCompleted then
let __stack_yield_fin = ResumableCode.Yield().Invoke(&sm)
__stack_condition_fin <- __stack_yield_fin
if __stack_condition_fin then
awaiter.GetResult()
else
sm.Data.MethodBuilder.AwaitUnsafeOnCompleted(&awaiter, &sm)
__stack_condition_fin
else
let vtask = compensation ()
let mutable awaiter = vtask.GetAwaiter()
let cont =
TaskResumptionFunc<'TOverall>(fun sm ->
awaiter.GetResult() |> ignore
true)
// shortcut to continue immediately
if awaiter.IsCompleted then
cont.Invoke(&sm)
else
sm.ResumptionDynamicInfo.ResumptionData <- (awaiter :> ICriticalNotifyCompletion)
sm.ResumptionDynamicInfo.ResumptionFunc <- cont
false)
)
```
Test code is:
```fsharp
[]
let``task should propagate exception if thrown in DisposeAsync``() = task {
let disposable = { new IAsyncDisposable with
member _.DisposeAsync() = ValueTask(task {
do! Task.Delay(1)
failwith "boom"
})
}
let! _ = Assert.ThrowsAsync(fun () -> task {
use _ = disposable
()
})
()
}
```
Current behavior:
```
Assert.Throws() Failure
Expected: typeof(System.Exception)
Actual: (No exception was thrown)
```
Contributor guide
Assessment
This issue has not been assessed yet.