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

Uninterruptible fibers' bracket handler marked as 'Killed' instead of 'Completed' #174

Open
felixSchl opened this issue May 20, 2019 · 1 comment

Comments

@felixSchl
Copy link

module Test.Main
  ( main
  ) where

import Prelude

import Control.Monad.Fork.Class (BracketCondition(..), bracket, fork, uninterruptible)
import Data.Time.Duration (Milliseconds(..))
import Debug.Trace (traceM)
import Effect (Effect)
import Effect.Aff (delay, killFiber, launchAff_)
import Effect.Class (liftEffect)
import Effect.Class.Console as Console
import Effect.Exception (error)

atomic action postAction =
  bracket (pure unit) (\a b ->
    case a of
      Completed x ->
        postAction
      Failed e ->
        traceM "failed"
      Killed e ->
        traceM "killed"
  ) (\_-> action)

main :: Effect Unit
main = do
 launchAff_ do

   fiber <- fork do
     atomic
      (uninterruptible do
          delay $ 15.0 # Milliseconds
          liftEffect $ Console.log "hi"
      )
      (liftEffect $ Console.log "here")

   delay $ 10.0 # Milliseconds
   killFiber (error "asdf") fiber

The above code only traces "killed", although it should most likely have run the postAction handler, thus printing "here" to the console.

@felixSchl
Copy link
Author

felixSchl commented May 20, 2019

This is undoubtedly not a proper solution, but it does seem to work at face value:

              if (interrupt && interrupt !== tmp && bracketCount === 0) {
                if (util.isLeft(step)) {
                  step = attempt._1.killed(util.fromLeft(interrupt))(attempt._2);
                } else {
                  step = attempt._1.completed(util.fromRight(step))(attempt._2);
                }

I suppose it comes down to being able to ask "was this fiber uninterruptible?" based on the current state.

EDIT: this breaks a lot of tests, and is outright wrong.

felixSchl pushed a commit to dn3010/purescript-aff that referenced this issue May 20, 2019
felixSchl pushed a commit to dn3010/purescript-aff that referenced this issue May 20, 2019
felixSchl pushed a commit to dn3010/purescript-aff that referenced this issue May 21, 2019
felixSchl pushed a commit to dn3010/purescript-aff that referenced this issue May 21, 2019
felixSchl pushed a commit to dn3010/purescript-aff that referenced this issue May 22, 2019
Fix purescript-contrib#174 by guaranteeing that iff a bracket body handler completes
uninterrupted (e.g. if masked, or simply no interrupts,) it *will*
call the 'completed' handler of the surrounding bracket.

This effectively allows writing code like this:

```purescript
bracket (liftEffect $ Ref.new Nothing)
  { completed: \a ref ->
      liftEffect $ Ref.write (Just a) ref
  , killed: \_ _ -> ...
  , failed: \_ _ -> ...
  } \_ -> action
```

We can effectively reason about the fact that, should `action` ever
complete, we *will* run the completion handler. Currently, if `action`
is masked, and thus guaranteed to succeed, we invoke the 'killed' handler,
thus denying us the possibily of obtaining 'a', even if it was produced.
felixSchl pushed a commit to dn3010/purescript-aff that referenced this issue May 22, 2019
Fix purescript-contrib#174 by guaranteeing that iff a bracket body handler completes
uninterrupted (e.g. if masked, or simply no interrupts,) it *will*
call the 'completed' handler of the surrounding bracket.

This effectively allows writing code like this:

```purescript
bracket (liftEffect $ Ref.new Nothing)
  { completed: \a ref ->
      liftEffect $ Ref.write (Just a) ref
  , killed: \_ _ -> ...
  , failed: \_ _ -> ...
  } \_ -> action
```

We can effectively reason about the fact that, should `action` ever
complete, we *will* run the completion handler. Currently, if `action`
is masked, and thus guaranteed to succeed, we invoke the 'killed' handler,
thus denying us the possibily of obtaining 'a', even if it was produced.
JCSanPedro added a commit to dn3010/purescript-aff that referenced this issue Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant