fslang-suggestions
fslang-suggestions copied to clipboard
fsharp
Support DisposeAsync in async computation expression
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:
- [x] This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
- [x] I have searched both open and closed suggestions on this site and believe this is not a duplicate
- [x] 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:
- [x] This is not a breaking change to the F# language design
- [x] I or my company would be willing to help implement and/or test this
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.
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
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.
@baronfel Yes, and getting rid of the Async.RunSynchronously would require implementing #853.
@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.
@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>) = ...
tryAsyncFinallywon'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.
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.
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?
@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
I'm marking this as approved - we should sort this out one way or another, and also document the necessary workaround
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)
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?
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.
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.
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
}
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.
I just tested this a bit more extensively to make sure:
- DisposeAsync is chosen over Dispose if a type implements both
IDisposableandIAsyncDisposable - 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.
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.
I think we should do this. Async should respect IAsyncDisposable. Marking as approved-in-principle.
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.
