Support DisposeAsync in async computation expression

[MaxDeg]

Support DisposeAsync in async/task computation expression

I propose we add support for the IAsyncDisposable interface in async computation.
The addition of a new keyword use! seems a little bit weird because it would be only for async builder. I suppose we could try to implement it in the current Using method of the builder

The existing way of approaching this problem in F# is calling the DisposeAsync manually and handling the error case by hands.

Pros and Cons

The advantages of making this adjustment to F# are ...
If IAsyncDisposable interface is available we should use it instead of the synchronous one.
The usage of StreamWriter on AspNet Core (3.x) reponse body with the keyword use will raise exception due to synchronous flush

ASP.NET Core : Synchronous operations are disallowed. Call FlushAsync or set AllowSynchronousIO to true instead

The disadvantages of making this adjustment to F# are ..
Don't see any

Extra information

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

Related suggestions: (put links to related suggestions here)

Affidavit (please submit!)

Please tick this by placing a cross in the box:

• This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
• I have searched both open and closed suggestions on this site and believe this is not a duplicate
• This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.

Please tick all that apply:

• This is not a breaking change to the F# language design
• I or my company would be willing to help implement and/or test this

[cartermp]

The challenge to this is that IAsyncDisposable is not available in .NET Standard 2.0, so we can't depend on the type directly in FSharp.Core. The type shape would have to be recognized in the compiler to have this work, which makes this much more costly to implement. Either that, or FSharp.Core adds a .NET Standard 2.1 target, which we sorta don't want to do at this stage.

[0x53A]

Could this be prototyped outside of FSharp.Core with an Extension Method to the async builder? https://docs.microsoft.com/en-us/dotnet/fsharp/language-reference/computation-expressions#extending-existing-builders-with-new-custom-operations

[baronfel]

It would take some work to do it right, but the CE mechanism is flexible enough to support extensions here. Here's a dumb-but-I-think-correct implementation that calls IAsyncDisposable.DisposeAsync in a blocking manner (to more easily fit in with the current signatures):

open System
open System.Threading.Tasks
open System.Threading

type TrackingDisposer() = 
    let mutable disposed = false

    member _.Disposed = disposed

    interface IAsyncDisposable with
      member __.DisposeAsync() =
       disposed <- true
       ValueTask()


type AsyncBuilder with
  member builder.Using(resource:#IAsyncDisposable, f: #IAsyncDisposable -> Async<'a>) = 
    let mutable x = 0
    let disposeFunction _ =
        if Interlocked.CompareExchange(&x, 1, 0) = 0 then
            let t = resource.DisposeAsync().AsTask() |> Async.AwaitTask
            Async.RunSynchronously t

    async.TryFinally(f resource, disposeFunction)

let test () = 
    let d = TrackingDisposer()

    async {
        use _ignored = d
        return ()
    }
    |> Async.RunSynchronously

    d.Disposed

if you call test you'll note that the dispose function was in fact called.

[Tarmil]

@baronfel Yes, and getting rid of the Async.RunSynchronously would require implementing #853.

[dbrattli]

@baronfel But if I try this with my own builder then I get the error "The type 'IAsyncDisposable' is not compatible with the type 'System.IDisposable'F# Compiler(1)" when I try to use! it.

UPDATE: Please ignore, I was testing inside an Expecto testAsync which is not the same as async.

[Arshia001]

@Tarmil

getting rid of the Async.RunSynchronously would require implementing #853.

does it really require #853? You'd normally make a using like this:

let tryFinally (tryPart: unit -> Task<'a>) (finallyPart: unit -> unit) = // ...
let using body disposable =
    tryFinally (fun () -> body disposable) (fun () -> disposable.Dispose())

To make IAsyncDisposable work, one could theoretically use functions with signatures similar to these:

let tryAsyncFinally (tryPart: unit -> Task<'a>) (finallyPart: unit -> Task<unit>) = // ...
let usingAsync body asyncDisposable =
    tryAsyncFinally (fun () -> body asyncDisposable) (fun () -> asyncDisposable.DisposeAsync())

tryAsyncFinally won't be accessible inside CE's until #853 is implemented, but it doesn't need to be accessible for it to work here.

I actually once added an implementation of this in TaskBuilder.fs. hoping to be able to add an overload for the Using method inside the builder. However, that's where I got stuck. The compiler would refuse to compile this code and complained that these two methods have the same signature (which they don't, but maybe I don't understand the F# type system completely?):

member __.Using(disposable : #IDisposable, body : #IDisposable -> Step<'u>) = ...
member __.Using(disposable : #IAsyncDisposable, body : #IAsyncDisposable -> Step<'u>) = ...

[Tarmil]

tryAsyncFinally won't be accessible inside CE's until #853 is implemented

Yes, that's what I meant 🙂

The compiler would refuse to compile this code and complained that these two methods have the same signature (which they don't, but maybe I don't understand the F# type system completely?):

member __.Using(disposable : #IDisposable, body : #IDisposable -> Step<'u>) = ...
member __.Using(disposable : #IAsyncDisposable, body : #IAsyncDisposable -> Step<'u>) = ...

The above are actually syntactic sugar for the following:

member __.Using<'t, 'u when 't :> IDisposable>(disposable: 't, body: 't -> Step<'u>) = ...
member __.Using<'t, 'u when 't :> IAsyncDisposable>(disposable: 't, body: 't -> Step<'u>) = ...

and method resolution doesn't take type constraints into consideration to choose an overload.

[Arshia001]

method resolution doesn't take type constraints into consideration to choose an overload.

So basically we can't have different overloads of Using (even though overloads are supported in CE builders IIRC) and have to wait for language support just so we can use a method with a new name? That's... rather unfortunate.

[andagr]

A related problem is that the recommended method for sending events to Azure Event Hubs nowadays is using EventHubProducerClient, which only implements IAsyncDisposable, according to the official docs. There is another client called EventHubClient, but in the official docs there is now a big warning at the top stating that this is "old", which might mean it will be deprecated and not further developed?

[cartermp]

@dsyme now that we have APIs depending on this type (and pattern, really) it seems like the right thing to put on the docket for F# vNext

[forki]

yeah that situation with eventhubs is really worrying.

[dsyme]

I'm marking this as approved - we should sort this out one way or another, and also document the necessary workaround

[baronfel]

Noting here that GRPC (which is very well supported and promoted in .net now) makes use of IAsyncDisposable so there's a growing gap here in correctly consuming these libraries (without hacks like calling .GetAwaiter().GetResult() on the DisposeAsync() members of a type)

[NinoFloris]

Most natural option seems to be to support use! as a desugaring of TryFinally and Bind. This would only require us to extend TryFinally somewhat to support a finallyBody with an M<'T> return type (described in #853).

At a later moment we could always decide to add a UsingBind method to override the desugaring if behavior must for some reason deviate from a regular bind (or support for IAsyncDisposable restricted to just use! and not do!). I don't see a lot of value in that yet.

@dsyme thoughts?

[dsyme]

Most natural option seems to be to support use! as a desugaring of TryFinally and Bind. This would only require us to extend TryFinally somewhat to support a finallyBody with an M<'T> return type (described in #853).

This would need overloading on TryFInally? Which I think will always cause problems.

Diving straight in and supporting a TryFinallyBind might make more sense - using it if there is any (syntactic) binding in the finally section.

[NinoFloris]

I was thinking about M<'T> being an alternative to the unit variant, considering zero should be implementable for any monad, much like TryWith which already supports exn -> M<'T>.

What I'd like to prevent is a proliferation of new methods that must all be implemented to provide a generally expected set of functionality.

IAsyncDisposable/M<'T> support will be expected and ideally any try finally form can be satisfied by implementing a single method.

[et1975]

what's annoying is that finally block is not even considered to be inside the CE:

task {
    let sender = client.Value.CreateSender topic
    try
        do! // stuff
    finally
        do! sender.DisposeAsync() // This construct may only be used within computation expressions
}

[Happypig375]

@et1975 That's #853 as mentioned above.

[cataggar]

With F# 6 task support, I am able to use an IAsyncDisposable with task, but not async. An example is:

[<EntryPoint>]
let main argv =
    let t = task {
        use serviceBus = new ServiceBusClient(connString)
        // other code
        return 0
    }
    t.Result

The ServiceBusClient only implements IAsyncDisposable.

[asik]

I just tested this a bit more extensively to make sure:

• DisposeAsync is chosen over Dispose if a type implements both IDisposable and IAsyncDisposable
• DisposeAsync is awaited and not called in a blocking way

This all seems to work well now in task CEs, so maybe this should be closed? If the issue remains open this can be misleading.

[cartermp]

I will leave this to @dsyme if this should be a considered a "no" for now. The original filing of the issue was before task was implemented in FSharp.Core and is still relevant for async CEs and any alterntative task CE that someone may prefer to use. But if it works as expected with a built-in construct I don't personally see a reason to implement this in other constructs.

[dsyme]

I think we should do this. Async should respect IAsyncDisposable. Marking as approved-in-principle.

[TheAngryByrd]

I wanted to mention I did a naive implementation over here in FsToolkit. Not sure what holes this implementation has, would love some feedback if possible.