Gabriella439 / pipes-concurrency

Concurrency for the pipes ecosystem
BSD 3-Clause "New" or "Revised" License
43 stars 12 forks source link

Abstract from pipes #41

Open haasn opened 7 years ago

haasn commented 7 years ago

It would seem that the entire library is mostly non-dependent on pipes except for the fromInput/toOutput helpers.

Especially when planning to use something like streaming, it can be bother to pull in all of pipes as well just for the non-pipe-dependent parts of this library, which IMO generalize well to other streaming-like abstractions (and even manual loops).

I think it would be doable to refactor those two functions in order to stub out yield / await and pass it in as a function. Pipes people can still use it like fromInput await inp and toOutput yield out.

Gabriella439 commented 7 years ago

I was originally going to do this until this issue occurred: https://github.com/Gabriel439/Haskell-Pipes-Concurrency-Library/issues/27

The core feature of the library is the ability to gracefully handle deadlocks and that stopped working in GHC 7.8.2, so I didn't feel comfortable with spinning off a smaller library when the core functionality was still broken

haasn commented 7 years ago

Ah, yeah; I did run into that myself but solved it by adding a performGC call when my threads exit. Is it still broken in GHC 7.8+ if I do that?

Gabriella439 commented 7 years ago

Not necessarily, but it will make the problem less likely. The example in #27 still fails, for example

ivan-m commented 7 years ago

Would you consider a main library exporting only withSpawn and exporting spawn and spawn' from an .Internal module?

(Trying to read through the logic in spawn', I'm unsure how much of that is trying to deal with the non-bracketed issue and how much could thus be avoided from a safe subset.)

Gabriella439 commented 7 years ago

I don't think forcing the use of withSpawn would fix the problem. For example, this still fails:

import Pipes.Concurrent

main = withSpawn Unbounded (\(_, input) -> atomically (recv input) )

... with:

test: thread blocked indefinitely in an STM transaction
ivan-m commented 7 years ago

Does that mean that the actual spawning abstraction itself is unsafe on its own?

Gabriella439 commented 7 years ago

Yes. That's the root of the problem, as described in issue #27

rainbyte commented 4 years ago

Now that #27 is already closed, is it possible to work in this issue? If that is the case I can try to make a PR.

Gabriella439 commented 4 years ago

@rainbyte: Yeah, this issue should be good to go now