Allow using unliftio-core instead of monad-control

[andremarianiello]

Just a little experiment to see what it would look like if monad-control was replaced

[andremarianiello]

@harendra-kumar if you are interested, please take a look. I haven't done anything with the tests or benchmarks, just made it compile.

[harendra-kumar]

The overall change looks pretty simple. I am wondering if we can restrict the abstractions to this file, and be able to switch between monad-control and unliftIO by just using a CPP macro. For example, we can define:

#ifdef UNLIFT_IO
type MonadControlIO m = MonadUnliftIO m
#else
type MonadControlIO m = (MonadIO m, MonadBaseControl IO m)
#endif

type MonadAsync m = (MonadControlIO m, MonadThrow m)

We can have similar definitions for RunInIO, runInIO and withRunIO (it will be control in case of monad-control).

With these changes we should only be importing this file everywhere rather than importing monad-control or unliftio. And then we can commit this change easily because there are no real changes to the code, its just a refactor to allow unliftio.

[andremarianiello]

Yup, sounds easy enough. I should be able to do that this weekend, if not sooner.

[harendra-kumar]

I merged monad-base branch to master. You can base it on master now.

[andremarianiello]

@harendra-kumar I rebased onto master and added the CPP flag handling to swap between unliftio-core and monad-control. Streamly.Internal.Control.Concurrent could be reorganized and could use some better docs, but I think this is the general approach you were looking for. Thoughts?

[harendra-kumar]

Nice and clean @andremarianiello . Thank you.

[harendra-kumar]

CIs look good. Merging it.

Thanks @andremarianiello

[hasufell]

Uhm... cabal flags should never affect external API.

Cabal currently does not allow setting flags on dependent packages from within a .cabal file. This means that people will have to rely on cabal.project, which neither works for transitive deps correctly, nor for packages you install directly from hackage.

That said, I too prefer MonadUnliftIO.

[harendra-kumar]

This is an experimental feature.

[hasufell]

This is an experimental feature.

That isn't quite the point. Flags that cause public API to change can make packages on hackage become unbuildable.

[harendra-kumar]

How does that happen? All flags may not be for end user consumption. We may have private build flags in the package e.g. we have flags to use for dev testing and benchmarking, some of these flags may alter the APIs or behavior of the package. It would be very restrictive if flags are only meant for API users' consumption.

[hasufell]

How does that happen?

type MonadAsync m = (MonadUnliftIO m, MonadThrow m)

vs

type MonadAsync m = (MonadIO m, MonadBaseControl IO m, MonadThrow m)

MonadAsync is exported from Streamly.Prelude.

Now, a downstream user may use the flag use-unliftio and develop their package against this MonadAsync type, let's call it package streamly-foo. Another user who has not set use-unliftio develops another package and depends on streamly and now wants to depend on streamly-foo too. That now may not work, as the class constraints across the codebase differ. The only option the user now has is:

• set use-unliftio in their cabal.project and rewrite their entire package. But that doesn't work for hackage packages, because cabal.project is not part of hackage packages and not considered by cabal/stack build for dependencies

The package streamly-foo may now be unbuildable for a large set of users.

---

That aside, I believe streamly should switch unconditionally to MonadUnliftIO, also see https://www.snoyman.com/blog/2018/02/conduitpocalypse/

In my own codebase, I'm already unable to use MonadBaseControl, because of the removed ResourceT instance.

[harendra-kumar]

Now, a downstream user may use the flag use-unliftio and develop their package against this MonadAsync type

That is the whole point of confusion, this flag is experimental and for private use only, it is not a released feature, therefore, you are not supposed to use this flag in a package and upload that package to hackage. For general use only monad-control is supported. This was implemented as a proof of concept to know what all needs to be changed if we have to change from monad-control to unliftio. It can be used in private projects though.

[hasufell]

I see, is there an ETA on when streamly switches to unliftio?

[harendra-kumar]

In my own codebase, I'm already unable to use MonadBaseControl, because of the snoyberg/conduit#404.

See this #395 (comment) to use resourcet.

I am not sure about the ETA, we are separating the serial streams from concurrent ones in a separate package possibly without dependency on monad-control. Will think about this after that.

[hasufell]

In my own codebase, I'm already unable to use MonadBaseControl, because of the snoyberg/conduit#404.

See this #395 (comment) to use resourcet.

I'm not sure I want to do that since snoyman's blog post indicated that the old runResourceT was unsafe:

In previous versions of the resourcet package, if you register a cleanup action which throws an exception itself, the exception would be swallowed. In this new release, any exceptions thrown during cleanup will be rethrown by runResourceT.