Skip to content

Pass cancellation tokens and tasks to OperationCanceledException and TaskCanceledException #2770

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

Merged
merged 3 commits into from
Apr 3, 2017
Merged

Pass cancellation tokens and tasks to OperationCanceledException and TaskCanceledException #2770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d71b02b
Initial attempt at passing in cancellation tokens/tasks to OperationC…
saul Apr 1, 2017
07747ba
Updated remaining references to OperationCanceledException
saul Apr 1, 2017
42cf793
Fix Reactor compilation
saul Apr 1, 2017
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
2 changes: 1 addition & 1 deletion src/absil/illib.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ module Cancellable =
let token () = Cancellable (fun ct -> ValueOrCancelled.Value ct)

/// Represents a canceled computation
let canceled() = Cancellable (fun _ -> ValueOrCancelled.Cancelled (new OperationCanceledException()))
let canceled() = Cancellable (fun ct -> ValueOrCancelled.Cancelled (new OperationCanceledException(ct)))

/// Catch exceptions in a computation
let private catch (Cancellable e) =
Expand Down
20 changes: 10 additions & 10 deletions src/fsharp/FSharp.Core/control.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ namespace Microsoft.FSharp.Control

// Call the cancellation continuation
let cancelT (args:AsyncParams<_>) =
args.aux.ccont (new OperationCanceledException())
args.aux.ccont (new OperationCanceledException(args.aux.token))

// Build a primitive without any exception of resync protection
//
Expand Down Expand Up @@ -1541,8 +1541,8 @@ namespace Microsoft.FSharp.Control
let continuation (completedTask : Task<_>) : unit =
args.aux.trampolineHolder.Protect((fun () ->
if completedTask.IsCanceled then
if useCcontForTaskCancellation then args.aux.ccont(new OperationCanceledException())
else args.aux.econt (ExceptionDispatchInfo.Capture(new TaskCanceledException()))
if useCcontForTaskCancellation then args.aux.ccont(new OperationCanceledException(args.aux.token))
Copy link
Member

Choose a reason for hiding this comment

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

@saul @dsyme this is incorrect. You are passing the async cancellation token which may not be related to the cancellation of the task we're binding on.

Copy link
Member

Choose a reason for hiding this comment

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

@saul @matthid this line should be reverted to the original (unless there is a generic way to obtain a cancellation token from arbitrary tasks which I'm not aware of)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok added in #3255

Copy link
Contributor

Choose a reason for hiding this comment

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

@eiriktsarpalis @matthid What is this were if args.aux.token.IsCancellationRequested then new OperationCanceledException(args.aux.token) else new OperationCanceledException() ?

I suspect a lot of corresponding C# code would do the same thing: if a cancellation continuation needs to be converted into a OCE, then you would attach the current cancellation token in scope to the OCE, at least on the condition that that the current cancellation token in scope is in the cancelled state,

else args.aux.econt (ExceptionDispatchInfo.Capture(new TaskCanceledException(completedTask)))
elif completedTask.IsFaulted then
args.aux.econt (MayLoseStackTrace(completedTask.Exception))
else
Expand All @@ -1555,8 +1555,8 @@ namespace Microsoft.FSharp.Control
let continuation (completedTask : Task) : unit =
args.aux.trampolineHolder.Protect((fun () ->
if completedTask.IsCanceled then
if useCcontForTaskCancellation then args.aux.ccont (new OperationCanceledException())
else args.aux.econt (ExceptionDispatchInfo.Capture(new TaskCanceledException()))
if useCcontForTaskCancellation then args.aux.ccont (new OperationCanceledException(args.aux.token))
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

else args.aux.econt (ExceptionDispatchInfo.Capture(new TaskCanceledException(completedTask)))
elif completedTask.IsFaulted then
args.aux.econt (MayLoseStackTrace(completedTask.Exception))
else
Expand Down Expand Up @@ -1630,7 +1630,7 @@ namespace Microsoft.FSharp.Control
match !timer with
| None -> ()
| Some t -> t.Dispose()
aux.trampolineHolder.Protect(fun () -> savedCCont(new OperationCanceledException())) |> unfake
aux.trampolineHolder.Protect(fun () -> savedCCont(new OperationCanceledException(aux.token))) |> unfake
),
null)
let mutable edi = null
Expand Down Expand Up @@ -1695,7 +1695,7 @@ namespace Microsoft.FSharp.Control
Async.Start (async { do (ccont e |> unfake) })

// register cancellation handler
let registration = aux.token.Register(fun () -> cancel (OperationCanceledException()))
let registration = aux.token.Register(fun () -> cancel (OperationCanceledException(aux.token)))

// run actual await routine
// callback will be executed on the thread pool so we need to use TrampolineHolder.Protect to install trampoline
Expand Down Expand Up @@ -1746,7 +1746,7 @@ namespace Microsoft.FSharp.Control
match !rwh with
| None -> ()
| Some rwh -> rwh.Unregister(null) |> ignore)
Async.Start (async { do (aux.ccont (OperationCanceledException()) |> unfake) }))
Async.Start (async { do (aux.ccont (OperationCanceledException(aux.token)) |> unfake) }))

and registration : CancellationTokenRegistration = aux.token.Register(cancelHandler, null)

Expand Down Expand Up @@ -1842,7 +1842,7 @@ namespace Microsoft.FSharp.Control
// Register the result. This may race with a successful result, but
// ResultCell allows a race and throws away whichever comes last.
once.Do(fun () ->
let canceledResult = Canceled (OperationCanceledException())
let canceledResult = Canceled (OperationCanceledException(cancellationToken))
resultCell.RegisterResult(canceledResult,reuseThread=true) |> unfake
)
| Some cancel ->
Expand Down Expand Up @@ -2007,7 +2007,7 @@ namespace Microsoft.FSharp.Control
event.RemoveHandler(del)
// Register the result. This may race with a successful result, but
// ResultCell allows a race and throws away whichever comes last.
once.Do(fun () -> resultCell.RegisterResult(Canceled (OperationCanceledException()),reuseThread=true) |> unfake)
once.Do(fun () -> resultCell.RegisterResult(Canceled (OperationCanceledException(token)),reuseThread=true) |> unfake)
| Some cancel ->
// If we get an exception from a cooperative cancellation function
// we assume the operation has already completed.
Expand Down
2 changes: 1 addition & 1 deletion src/fsharp/vs/Reactor.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ type Reactor() =
with e -> e |> AsyncUtil.AsyncException

resultCell.RegisterResult(result)),
ccont=(fun () -> resultCell.RegisterResult (AsyncUtil.AsyncCanceled(OperationCanceledException())) )
ccont=(fun () -> resultCell.RegisterResult (AsyncUtil.AsyncCanceled(OperationCanceledException(ct))) )

)
return! resultCell.AsyncResult
Expand Down