Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Change cancellation test to not depend on how AwaitTask works #7467

Merged
merged 2 commits into from
Sep 27, 2019
Merged

Change cancellation test to not depend on how AwaitTask works #7467

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c322b87
Change cancellation test to not depend on how AwaitTask works
Frassle Aug 29, 2019
75500f1
Add test that AwaitTask ignore async cancellation
Frassle Aug 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 32 additions & 5 deletions tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Control/AsyncType.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,24 +145,26 @@ type AsyncType() =
[<Test>]
member this.StartAsTaskCancellation () =
let cts = new CancellationTokenSource()
let tcs = TaskCompletionSource<unit>()
let mutable spinloop = true
let doSpinloop () = while spinloop do ()
let a = async {
cts.CancelAfter (100)
do! tcs.Task |> Async.AwaitTask }
doSpinloop()
}
#if !NET46
let t : Task<unit> =
#else
use t : Task<unit> =
#endif
Async.StartAsTask(a, cancellationToken = cts.Token)

// Should not finish
// Should not finish, we don't eagerly mark the task done just because it's been signaled to cancel.
try
let result = t.Wait(300)
Assert.IsFalse (result)
with :? AggregateException -> Assert.Fail "Task should not finish, jet"
with :? AggregateException -> Assert.Fail "Task should not finish, yet"

tcs.SetCanceled()
spinloop <- false

try
this.WaitASec t
Expand All @@ -172,6 +174,31 @@ type AsyncType() =
| _ -> reraise()
Assert.IsTrue (t.IsCompleted, "Task is not completed")

[<Test>]
member this.``AwaitTask ignores Async cancellation`` () =
let cts = new CancellationTokenSource()
let tcs = new TaskCompletionSource<unit>()
let innerTcs = new TaskCompletionSource<unit>()
let a = innerTcs.Task |> Async.AwaitTask

Async.StartWithContinuations(a, tcs.SetResult, tcs.SetException, ignore >> tcs.SetCanceled, cts.Token)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit bogus to me you await the task here and use setresult as continuation. But if the task finished setresult is already called and the continuation will throw

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah you have two tcs so this is basically just StartAsTask or very similar

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tcs vs innerTcs, this is setting the tcs result, while the task that we AwaitTask on is from innerTcs.


cts.CancelAfter(100)
try
let result = tcs.Task.Wait(300)
Assert.IsFalse (result)
with :? AggregateException -> Assert.Fail "Should not finish, yet"

innerTcs.SetResult ()

try
this.WaitASec tcs.Task
with :? AggregateException as a ->
match a.InnerException with
| :? TaskCanceledException -> ()
| _ -> reraise()
Assert.IsTrue (tcs.Task.IsCompleted, "Task is not completed")

[<Test>]
member this.StartTask () =
let s = "Hello tasks!"
Expand Down