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.

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

finally! #651

Open
dsyme opened this issue Mar 6, 2018 · 14 comments
Open

finally! #651

dsyme opened this issue Mar 6, 2018 · 14 comments

Comments

@dsyme
Copy link
Collaborator

dsyme commented Mar 6, 2018

I propose we add try .. finally! ... to allow finally actions to be monadic.

The current computation-expression desugaring of try .. finally ... allows only a unit -> unit action in the compensation branch.. However there are cases where an asynchronous or other monadic imperative action makes sense in exception compensation, e.g. using AsyncSeq:

        asyncSeq { 
              try
                do! Async.Sleep 2000; 
                yield UpdateItemsReceived [| { Text = "AsyncLoad1"; Description = "AsyncDescription1"  } |]
                do! Async.Sleep 2000; 
                yield UpdateItemsReceived [| { Text = "AsyncLoad2"; Description = "AsyncDescription2"  } |]
                do! Async.Sleep 2000; 
              finally!
                yield UpdateItemsComplete
            }

The current way of doing this in F# is some mess like this:

    asyncSeq { 
            do! Async.Sleep 2000; 
            yield UpdateItemsReceived [| { Text = "AsyncLoad1"; Description = "AsyncDescription1"  } |]
            do! Async.Sleep 2000; 
            yield UpdateItemsReceived [| { Text = "AsyncLoad2"; Description = "AsyncDescription2"  } |]
            do! Async.Sleep 2000; 
        }
   |> (fun a -> AsyncSeq.tryFinallyMonadic(a, fun () -> ...))

The same effect can also currently be achieved with a custom operation, something like this, but just supporting finally! looks more sensible:

    asyncSeq { 
          try
            do! Async.Sleep 2000; 
            yield UpdateItemsReceived [| { Text = "AsyncLoad1"; Description = "AsyncDescription1"  } |]
            do! Async.Sleep 2000; 
            yield UpdateItemsReceived [| { Text = "AsyncLoad2"; Description = "AsyncDescription2"  } |]
            do! Async.Sleep 2000; 
            yieldFinally UpdateItemsComplete
        }

Pros and Cons

The advantages of making this adjustment to F# are orthogonality/simplicity.

The disadvantages of making this adjustment to F# are cost.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): S

Related Items

@dsyme
Copy link
Collaborator Author

dsyme commented Mar 6, 2018

Note that there is a question for async/asyncSeq about whether such operations would be performed if the operation is cancelled, and how that would be implemented. try/finally are executed during cancellation. The above was on the assumption that an exception or normal result had occurred....

@rmunn
Copy link

rmunn commented Mar 7, 2018

I can see two possible ways to handle cancellation in finally! blocks for asyncs:

  1. A finally! block is always run when the operation is cancelled, and it's up to the programmer to ensure that whatever he does in the finally! block will not cause problems if the operation is being cancelled. There is no way for him to query whether the finally! is running during a normal result, an exception, or a cancellation, therefore caveat scriptor.
  2. A finally! block is always run when the operation is cancelled, but the AsyncBuilder (and related builders like AsyncSeq) can provide some way to tell, inside the finally! block, whether a cancellation is happening. So the programmer can write something like:
asyncSeq {
  try
    ...
  finally!
    let token = getCancellationTokenForThisBlock()
    if token.IsCancellationRequested then
      doVitalCleanup()
    else
      yield! longRunningAsyncProcess()
      yield UpdateItemsComplete
      doVitalCleanup()
}

Note that I did not list "finally! blocks are not run during cancellation" as an option, because I believe those semantics would be wrong. If finally is always run during cancellation, then finally! should behave similarly.

And BTW, I like the issue title. 😄

@Alxandr
Copy link

Alxandr commented Mar 14, 2018

Why does it need finally!? Can't it just be another overload for the builder, so that if your finally block returns an instance of the monad it's used?

@Thorium
Copy link

Thorium commented Apr 17, 2018

Why not catch! for Async.catch and other context based error handling,
(like custom logging and the railway-oriented style)

Edit: with is overloaded keyword, also used in match ... with, so we can't use that with bang?
Basically finally! would be quite near to match!: dotnet/fsharp#4427

@Thorium
Copy link

Thorium commented Apr 17, 2018

There is no currently easy way to say "Await async whatever, and then log this message".
So to make a common logging for failures, companies have to do something like:

  • inline all the common logging. Code stays simple but compiler starts to be slow
  • Jet.com asyncOmega-style own builder to replace current async.
  • GResearch style Existential type to hide the result and say just deal with it.
  • Or use TypeShape (or reflection). The code will be even more scary

Clearly it's a problem. I think catch! and finally! would solve this, but internally the same problem has to be dealt with, right?

@xperiandri
Copy link

xperiandri commented Apr 19, 2018

Can't compiler just look into catch and finally blocks and do right job without any suffix?

@Thorium
Copy link

Thorium commented Apr 19, 2018

The more general problem is already that if you do a lot of code inside a computation expression (like async), you lose the interactive-driven-development as you cannot easily send a part of computation expression code to FSI.

@Lanayx
Copy link

Lanayx commented Jul 8, 2021

This seems to be required to implement native support for IAsyncDisposable

@Thorium
Copy link

Thorium commented Jul 8, 2021

IAsyncDisposable sounds kind of funny:
I thought the original idea of IDisposable was to not left resources hanging...then what is the purpose of async disposable?

@Lanayx
Copy link

Lanayx commented Jul 8, 2021

IAsyncDisposable sounds kind of funny:
I thought the original idea of IDisposable was to not left resources hanging...then what is the purpose of async disposable?

Same idea, but sometimes you need to call async methods during the cleanup

@kekyo
Copy link

kekyo commented Jul 26, 2021

I think and want this feature.
I wrote simulated asynchronous finally block in hand-made, I feel it's too hard to implements.

First version (simplicity but will be hard blocked):

https://github.com/kekyo/FSharp.Control.FusionTasks/blob/878abd7ca819cc45f4ffc975b64912c85c080c8b/FSharp.Control.FusionTasks/Infrastructures.fs#L195

Final result:

https://github.com/kekyo/FSharp.Control.FusionTasks/blob/11138a39d8a741b706d42d370a9787475a62e10b/FSharp.Control.FusionTasks/Infrastructures.fs#L162

They couldn't write it average level engineer...

@En3Tho
Copy link

En3Tho commented Aug 2, 2021

@dsyme
I've been hacking FSharp.Core a little bit for (well not exactly finally! feature, but allowing use to consume a synthetic IFSharpAsyncDisposable type which has a unit -> Async'unit Dispose method). The problem I've found is cancellation. Usually simple unit -> unit bindings are always called because they are not Delayed (Async.Delay) and thus not checked for cancellation. On the other hand those async Dispose had that check and in fact did not properly Dispose.

The possible hack was to either change CancellationToken in AsyncActivation.aux.Token for cconts or to add a boolean value to AsyncActivation indicating that we are now in "forced" ccont state.

So the problem is: how to deal with cancellation in this case? On the other hand computation is cancelled and it's somewhat logical to cancel all async computations, finally included. On the other hand it makes sense to force finally computations because user could take some kind of distributed lock in async operation and it needs to be freed and so on.

@En3Tho
Copy link

En3Tho commented Aug 4, 2021

ping @dsyme I've skimmed through the issue again and you've raised this exact topic in 2018. Funny how I just missed it. Can you please share your thoughts on how should we deal with cancellation in general? And I guess hacks mentioned above are not the really desired ones am I right?

@dsyme
Copy link
Collaborator Author

dsyme commented Apr 13, 2023

I've marked this approved-in-principle - I don't know why it wasn't approved

Can you please share your thoughts on how should we deal with cancellation in general?

@En3Tho My apologies for not replying to the above.

It would be great to get a PR together and discuss this in context

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants