Fsharpplus: IDisposable disposed prematurely in monad transformers

Created on 11 Jul 2019  路  9Comments  路  Source: fsprojects/FSharpPlus

Description

Monad transformers don't work nicely with "use" IDisposable (e.g. SqlCommand). Disposable object gets disposed prematurely.

Repro steps

Minimal code to reproduce below, reproduces with other monad transformers e.g. ResultT

type SomethingDisposiable() =
    interface IDisposable with 
        member __.Dispose() =
            printfn "I'm disposed!"

    member __.AsyncSomeOption() : Async<int option> =
        async { 
            printfn "I'm doing something async..."
            return Some 1
            }

let reproducePrematureDisposal : Async<int option> =
    OptionT.run <|
    monad {
        use somethingDisposable = new SomethingDisposiable()
        let! (result : int) = OptionT <| somethingDisposable.AsyncSomeOption()

        printfn "Unpacked async option: %A" result
        return result
    }

Expected behavior

expected output:

I'm doing something async...
I'm disposed!
Unpacked async option: 1

Actual behavior

actual output:

I'm disposed!
I'm doing something async...
Unpacked async option: 1

i.e. somethingDisposable already disposed by the time when AsyncSomeOption invoked.

Known workarounds

don't use monad transformers, use async builder, write explicit control flow for nested monadic results (Option<>, Result<>).

Related information

  • nuget package version: 1.1.0-CI00252
  • .NET Framework 4.7.2
  • Operating system: Windows 10
bug

Most helpful comment

@DunetsNM I've applied the fix and used your repro as test.

It would be nice to add more tests for cexp-monad-trans so feel free to submit more repros here and we can add them, or if you feel like creating a PR with more tests that's always welcome.

All 9 comments

Please let me know if this is a wrong way to use transformers, however it works fine if there are no disposables.

Very interesting repro !

I think it has to do with the Delay member which wasn't included in all monad transformers.
For instance ReaderT has it but not OptionT.

What I think it's happening is that the Delay from Async in not being invoked as OptionT doesn't have a Delay method which "links" with Async's Delay.

@DunetsNM regarding you last question:

Please let me know if this is a wrong way to use transformers, however it works fine if there are no disposables.

I can't tell you if it's wrong or is it right, AFAIK this is very first time we can start to experiment with disposables in generic Monad Transformers, so we're probably making history ;)

more likely it indicates that my functions are too large and need to be split :) anyway I'm proud to catch a runtime error in something very statically typed!

@DunetsNM Looks like I have a fix for this, but I wanted to confirm with you that your expected sequence is wrong:

You stated

expected output:

I'm doing something async...
I'm disposed!
Unpacked async option: 1

but my understanding is that the expected output should be:

I'm doing something async...
Unpacked async option: 1
I'm disposed!

Which is what happens if you use a normal async workflow.

Please confirm, and thanks again for the report.

This is probably correct if that's what normal async does. My expected output was just from the top of my head and I didn't really look at the async and how it works with disposables. Thanks

Thanks for the clarification.
Of course, there's not doubt that it should dispose at the end. I think that's the issue itself.

I'm adding all the missing methods to the transformers which fixes the issue, at least testing with this kind of samples pass, so I will add some tests similar to your repro.

The idea is to produce a nuget afterwards, since there is plenty of new functionality, so you'll be able to use this very soon.

@DunetsNM I've applied the fix and used your repro as test.

It would be nice to add more tests for cexp-monad-trans so feel free to submit more repros here and we can add them, or if you feel like creating a PR with more tests that's always welcome.

@gusty my disposable was a naked SqlConnection and I had only two occasions so far: Async<Option<>> and Async<Result<>> I will make a test for Result and maybe for not-happy-path e.g. if exception thrown midway

Was this page helpful?
0 / 5 - 0 ratings

Related issues

TOTBWF picture TOTBWF  路  7Comments

robkuz picture robkuz  路  5Comments

cmeeren picture cmeeren  路  7Comments

cannorin picture cannorin  路  3Comments

abelbraaksma picture abelbraaksma  路  3Comments