From 20145b6c2dc319654160f056db3ec43186e75a6e Mon Sep 17 00:00:00 2001 From: Bennj Date: Wed, 15 Feb 2017 18:23:50 -0500 Subject: [PATCH 1/3] WIP: Fix handling of `Task` thrown Exceptions Calling `xK.DoCont(ref wr, xT.Result)` when the `xT.Result` ends up resolving to an exception causes the resulting exception to be wrapped in an aggregate exception. This is undesired behavior. --- Libs/Hopac.Core/External/Tasks.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Libs/Hopac.Core/External/Tasks.cs b/Libs/Hopac.Core/External/Tasks.cs index abe23f8d..55d04660 100644 --- a/Libs/Hopac.Core/External/Tasks.cs +++ b/Libs/Hopac.Core/External/Tasks.cs @@ -108,7 +108,11 @@ internal sealed class TaskToJobAwaiter : Work { xK.DoHandle(ref wr, e); } internal override void DoWork(ref Worker wr) { - xK.DoCont(ref wr, xT.Result); + var xT = this.xT; + if (TaskStatus.RanToCompletion == xT.Status) + xK.DoCont(ref wr, xT.Result); + else + xK.DoHandle(ref wr, xT.Exception); } public void Ready() { Worker.ContinueOnThisThread(this.sr, this); From c9640efc32776eef6715aff5dc06673e36e1806f Mon Sep 17 00:00:00 2001 From: Marcus Griep Date: Thu, 16 Feb 2017 14:17:06 -0500 Subject: [PATCH 2/3] Add tests and update handling of tasks Ensures that cancelled tasks are handled in a consistent way, by passing along a `TaskCanceledException`. Also efficiently handles thrown exceptions by passing them directly to the handler rather than relying on `.Result` to re-throw the exception. --- Libs/Hopac.Core/External/Tasks.cs | 41 ++++++++++++++++++++----------- Tests/AdHocTests/TaskTests.fs | 26 ++++++++++++++++++++ 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/Libs/Hopac.Core/External/Tasks.cs b/Libs/Hopac.Core/External/Tasks.cs index 55d04660..50468382 100644 --- a/Libs/Hopac.Core/External/Tasks.cs +++ b/Libs/Hopac.Core/External/Tasks.cs @@ -32,7 +32,11 @@ private sealed class State : Work { } internal override void DoWork(ref Worker wr) { var yJ = this.yJ; - yJ.Do(yJ.xT.Result).DoJob(ref wr, yK); + var xT = yJ.xT; + if (TaskStatus.Faulted == xT.Status) + Handler.DoHandle(yK, ref wr, xT.Exception); + else + yJ.Do(yJ.xT.Result).DoJob(ref wr, yK); } internal void Ready() { Worker.ContinueOnThisThread(sr, this); @@ -73,10 +77,10 @@ private sealed class State : Work { internal override void DoWork(ref Worker wr) { var yJ = this.yJ; var uT = yJ.uT; - if (TaskStatus.RanToCompletion == uT.Status) - yJ.Do().DoJob(ref wr, yK); - else + if (TaskStatus.Faulted == uT.Status) Handler.DoHandle(yK, ref wr, uT.Exception); + else + yJ.Do().DoJob(ref wr, yK); } internal void Ready() { Worker.ContinueOnThisThread(sr, this); @@ -109,10 +113,10 @@ internal sealed class TaskToJobAwaiter : Work { } internal override void DoWork(ref Worker wr) { var xT = this.xT; - if (TaskStatus.RanToCompletion == xT.Status) - xK.DoCont(ref wr, xT.Result); - else + if (TaskStatus.Faulted == xT.Status) xK.DoHandle(ref wr, xT.Exception); + else + xK.DoCont(ref wr, xT.Result); } public void Ready() { Worker.ContinueOnThisThread(this.sr, this); @@ -150,10 +154,12 @@ internal sealed class TaskToJobAwaiter : Work { } internal override void DoWork(ref Worker wr) { var uT = this.uT; - if (TaskStatus.RanToCompletion == uT.Status) - uK.DoWork(ref wr); - else + if (TaskStatus.Faulted == uT.Status) uK.DoHandle(ref wr, uT.Exception); + else { + uT.Wait(); + uK.DoWork(ref wr); + } } internal void Ready() { Worker.ContinueOnThisThread(this.sr, this); @@ -207,7 +213,11 @@ internal sealed class TaskToAltAwaiter : Cont { cts.Dispose(); if (0 != picked) goto Done; - xK.DoCont(ref wr, xT.Result); + var xT = this.xT; + if (TaskStatus.Faulted == xT.Status) + xK.DoHandle(ref wr, xT.Exception); + else + xK.DoCont(ref wr, xT.Result); Done: return; } @@ -295,10 +305,13 @@ internal sealed class TaskToAltAwaiter : Cont { cts.Dispose(); if (0 != picked) goto Done; - if (TaskStatus.RanToCompletion == uT.Status) - uK.DoWork(ref wr); - else + var uT = this.uT; + if (TaskStatus.Faulted == uT.Status) uK.DoHandle(ref wr, uT.Exception); + else { + uT.Wait(); + uK.DoWork(ref wr); + } Done: return; } diff --git a/Tests/AdHocTests/TaskTests.fs b/Tests/AdHocTests/TaskTests.fs index b50fc54e..7889e10a 100644 --- a/Tests/AdHocTests/TaskTests.fs +++ b/Tests/AdHocTests/TaskTests.fs @@ -46,6 +46,32 @@ let run () = |> run |> testExpected [Ex] + let cancelled = TaskCanceledException() + + let tcs = TaskCompletionSource() + do Job.fromTask ^ fun () -> tcs.SetCanceled() ; tcs.Task + |> Job.catch + |> run + |> testExpected [cancelled] + + let tcs = TaskCompletionSource() + do Job.fromUnitTask ^ fun () -> tcs.SetCanceled() ; tcs.Task :> Task + |> Job.catch + |> run + |> testExpected [cancelled] + + let tcs = TaskCompletionSource() + do Job.fromTask ^ fun () -> tcs.SetException(Ex) ; tcs.Task + |> Job.catch + |> run + |> testExpected [Ex] + + let tcs = TaskCompletionSource() + do Job.fromUnitTask ^ fun () -> tcs.SetException(Ex) ; tcs.Task :> Task + |> Job.catch + |> run + |> testExpected [Ex] + do delayAndRaise 50 Ex <|> delayAndSet 203 ^ ref 1 |> Job.catch From 263ee1941221cdf2cdc47a23370f050517a3a783 Mon Sep 17 00:00:00 2001 From: Marcus Griep Date: Thu, 16 Feb 2017 14:59:30 -0500 Subject: [PATCH 3/3] Add more sequence tests --- Tests/AdHocTests/TaskTests.fs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/Tests/AdHocTests/TaskTests.fs b/Tests/AdHocTests/TaskTests.fs index 7889e10a..edc614f2 100644 --- a/Tests/AdHocTests/TaskTests.fs +++ b/Tests/AdHocTests/TaskTests.fs @@ -11,6 +11,7 @@ open System.Threading.Tasks open RunTask exception Ex +exception Ex2 let verify n c = printfn "%s %s" (if c then "Ok" else "FAILURE") n @@ -66,12 +67,38 @@ let run () = |> run |> testExpected [Ex] + let tcs1 = TaskCompletionSource() + let tcs2 = TaskCompletionSource() + do Job.fromTask ^ fun () -> tcs1.SetException(Ex) ; tcs1.Task + |> Job.bind ^ Job.liftTask ^ fun _ -> tcs2.SetException(Ex2) ; tcs2.Task + |> Job.catch + |> run + |> testExpected [Ex] + let tcs = TaskCompletionSource() do Job.fromUnitTask ^ fun () -> tcs.SetException(Ex) ; tcs.Task :> Task |> Job.catch |> run |> testExpected [Ex] + let tcs1 = TaskCompletionSource() + let tcs2 = TaskCompletionSource() + do 23 + |> Job.liftTask ^ fun _ -> tcs1.SetException(Ex) ; tcs1.Task + |> Job.bind ^ Job.liftTask ^ fun _ -> tcs2.SetCanceled() ; tcs2.Task + |> Job.catch + |> run + |> testExpected [Ex] + + let tcs1 = TaskCompletionSource() + let tcs2 = TaskCompletionSource() + do 23 + |> Job.liftTask ^ fun _ -> tcs1.SetCanceled() ; tcs1.Task + |> Job.bind ^ Job.liftTask ^ fun _ -> tcs2.SetException(Ex) ; tcs2.Task + |> Job.catch + |> run + |> testExpected [cancelled] + do delayAndRaise 50 Ex <|> delayAndSet 203 ^ ref 1 |> Job.catch