Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resource.finally drops writes #257

Closed
robrix opened this issue Oct 9, 2019 · 2 comments · Fixed by #268
Closed

Resource.finally drops writes #257

robrix opened this issue Oct 9, 2019 · 2 comments · Fixed by #268
Labels
bug Something isn't working
Milestone

Comments

@robrix
Copy link
Contributor

robrix commented Oct 9, 2019

λ runM (runResource (runState 'a' (modify (succ @Char) `finally` modify (succ . succ @Char))))
( 'b'
, ()
) 

See also this thread where @gelisam discusses the problem. Based on this, I think we’re going to want to change the type of finally (at a minimum).

cf #180

@robrix robrix added the bug Something isn't working label Oct 9, 2019
@robrix robrix added this to the 1.0 milestone Oct 9, 2019
@patrickt
Copy link
Collaborator

If we were to plumb a MonadCatch, MonadThrow, and MonadMask implementation through every carrier, we can use the finally from exceptions’s Control.Monad.Catch, which performs correctly:

>>> runM (runState 'a' (modify (succ @Char) `finally` modify (succ . succ @Char)))
('d',())

@lexi-lambda
Copy link

I already made these comments on slack, but I’ll make them here as well. My primary piece of advice is: you should reread my MonadBaseControl blog post! 😉 It has a section on how to make finally (and things like it) not discard state.

However, don’t despair: there actually is a way to produce a lifted version of finally that preserves all state changes. It can’t be done by lifting finally directly, but if we reimplement finally in terms of simpler lifted functions that are more amenable to lifting, we can produce a lifted version of finally that preserves all the state:

finally' :: MonadBaseControl IO m => m a -> m b -> m a
finally' ma mb = mask' $ \restore -> do
  a <- liftBaseWith $ \runInBase ->
    try (runInBase (restore ma))
  case a of
    Left e -> mb *> liftBase (throwIO (e :: SomeException))
    Right s -> restoreM s <* mb

You should be able to do the same thing in fused-effects. Also, I would recommend taking a look at the discussion in polysemy-research/polysemy#84, which discusses related problems in the context of bracket.

@robrix robrix mentioned this issue Oct 16, 2019
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants