{"action":"created","issue":{"url":"https://api.github.com/repos/scalaz/scalaz-stream/issues/333","labels_url":"https://api.github.com/repos/scalaz/scalaz-stream/issues/333/labels{/name}","comments_url":"https://api.github.com/repos/scalaz/scalaz-stream/issues/333/comments","events_url":"https://api.github.com/repos/scalaz/scalaz-stream/issues/333/events","html_url":"https://github.com/scalaz/scalaz-stream/issues/333","id":61877650,"number":333,"title":"Append is not associative with finalizers","user":{"login":"djspiewak","id":752,"avatar_url":"https://avatars.githubusercontent.com/u/752?v=3","gravatar_id":"","url":"https://api.github.com/users/djspiewak","html_url":"https://github.com/djspiewak","followers_url":"https://api.github.com/users/djspiewak/followers","following_url":"https://api.github.com/users/djspiewak/following{/other_user}","gists_url":"https://api.github.com/users/djspiewak/gists{/gist_id}","starred_url":"https://api.github.com/users/djspiewak/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/djspiewak/subscriptions","organizations_url":"https://api.github.com/users/djspiewak/orgs","repos_url":"https://api.github.com/users/djspiewak/repos","events_url":"https://api.github.com/users/djspiewak/events{/privacy}","received_events_url":"https://api.github.com/users/djspiewak/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/scalaz/scalaz-stream/labels/bug","name":"bug","color":"e11d21"},{"url":"https://api.github.com/repos/scalaz/scalaz-stream/labels/design/refactoring","name":"design/refactoring","color":"fbca04"}],"state":"open","locked":false,"assignee":{"login":"djspiewak","id":752,"avatar_url":"https://avatars.githubusercontent.com/u/752?v=3","gravatar_id":"","url":"https://api.github.com/users/djspiewak","html_url":"https://github.com/djspiewak","followers_url":"https://api.github.com/users/djspiewak/followers","following_url":"https://api.github.com/users/djspiewak/following{/other_user}","gists_url":"https://api.github.com/users/djspiewak/gists{/gist_id}","starred_url":"https://api.github.com/users/djspiewak/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/djspiewak/subscriptions","organizations_url":"https://api.github.com/users/djspiewak/orgs","repos_url":"https://api.github.com/users/djspiewak/repos","events_url":"https://api.github.com/users/djspiewak/events{/privacy}","received_events_url":"https://api.github.com/users/djspiewak/received_events","type":"User","site_admin":false},"milestone":{"url":"https://api.github.com/repos/scalaz/scalaz-stream/milestones/3","html_url":"https://github.com/scalaz/scalaz-stream/milestones/0.8.0","labels_url":"https://api.github.com/repos/scalaz/scalaz-stream/milestones/3/labels","id":1026357,"number":3,"title":"0.8.0","description":"","creator":{"login":"djspiewak","id":752,"avatar_url":"https://avatars.githubusercontent.com/u/752?v=3","gravatar_id":"","url":"https://api.github.com/users/djspiewak","html_url":"https://github.com/djspiewak","followers_url":"https://api.github.com/users/djspiewak/followers","following_url":"https://api.github.com/users/djspiewak/following{/other_user}","gists_url":"https://api.github.com/users/djspiewak/gists{/gist_id}","starred_url":"https://api.github.com/users/djspiewak/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/djspiewak/subscriptions","organizations_url":"https://api.github.com/users/djspiewak/orgs","repos_url":"https://api.github.com/users/djspiewak/repos","events_url":"https://api.github.com/users/djspiewak/events{/privacy}","received_events_url":"https://api.github.com/users/djspiewak/received_events","type":"User","site_admin":false},"open_issues":7,"closed_issues":0,"state":"open","created_at":"2015-03-17T23:08:30Z","updated_at":"2015-04-02T03:16:38Z","due_on":null,"closed_at":null},"comments":24,"created_at":"2015-03-15T18:16:23Z","updated_at":"2015-04-12T21:25:46Z","closed_at":null,"body":"```scala\r\n  property(\"associativity of append with finalizers\") = secure {\r\n    @volatile\r\n    var finalized = false\r\n\r\n    val p = emit(()) ++ (emit(()) onComplete eval_(Task delay { finalized = true }))\r\n    // val p = (emit(()) ++ emit(())) onComplete eval_(Task delay { finalized = true })\r\n\r\n    p.kill.run.run\r\n    finalized\r\n  }\r\n```\r\n\r\nIn its current configuration, the above test *fails*.  If I swap the commented lines, which is to say swap the associativity on the `++` operator, the test will pass.  In other words, binding a finalizer to a non-`halt` process *within* an append breaks the guarantees about finalizer evaluation.\r\n\r\nEven more seriously, the fact that there is an observable difference between the two `p` variants means that `++` is not associative.  As an extension of this, `flatMap` is also non-associative.  It was previously known that `flatMap` was non-associative in the presence of `njoin` (#274).  This test case shows that `njoin` is unnecessary to demonstrate failure of associativity with monadic joining.\r\n\r\nNeedless to say, this is an undesirable state of affairs.  It makes it difficult to reason about finalizers in general, and can mean that naively extracting a process definition into a variable (thus altering the associativity) can change the semantics.  Even worse, a given process definition will have different semantics depending on the associativity with which it is composed into another process!\r\n\r\nI believe all of these issues come down to a single design flaw: `append` and `onComplete` are implemented using the same mechanism (`onHalt`), which is overly general.  We need to split up `append` from finalization.  Doing this correctly should resolve the associativity of `++` issue described here (and by extension, `flatMap`).  Resolving the `njoin` associativity issues would require we take the further step of restricting finalizer processes to be of type `Process[F, Nothing]` (in fact, we probably need to do this even for `++` associativity, but I haven't thought that through).\r\n\r\nSo the tl;dr here is that the monadic laws do not hold for `Process` in the presence of finalizers.  Or rather, they do not hold in the presence of finalizers for a \"strict\" definition of process equality.  The above assumes we define two processes to be equal if they a) produce the same elements in order, *and* b) perform the same effects in order.  If we drop the second requirement, then magically the monadic laws (and `++` associativity!) holds once more.  However, I would argue that this weaker definition of process equality is somewhat useless, given the primary use-cases for *effectful* streams.\r\n\r\nAt the end of the day, we want to use `Process` as a way of reasonably managing effects, and it is difficult to reason about a \"monad\" for which the monadic laws do not always hold."},"comment":{"url":"https://api.github.com/repos/scalaz/scalaz-stream/issues/comments/92135638","html_url":"https://github.com/scalaz/scalaz-stream/issues/333#issuecomment-92135638","issue_url":"https://api.github.com/repos/scalaz/scalaz-stream/issues/333","id":92135638,"user":{"login":"pchlupacek","id":678560,"avatar_url":"https://avatars.githubusercontent.com/u/678560?v=3","gravatar_id":"","url":"https://api.github.com/users/pchlupacek","html_url":"https://github.com/pchlupacek","followers_url":"https://api.github.com/users/pchlupacek/followers","following_url":"https://api.github.com/users/pchlupacek/following{/other_user}","gists_url":"https://api.github.com/users/pchlupacek/gists{/gist_id}","starred_url":"https://api.github.com/users/pchlupacek/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pchlupacek/subscriptions","organizations_url":"https://api.github.com/users/pchlupacek/orgs","repos_url":"https://api.github.com/users/pchlupacek/repos","events_url":"https://api.github.com/users/pchlupacek/events{/privacy}","received_events_url":"https://api.github.com/users/pchlupacek/received_events","type":"User","site_admin":false},"created_at":"2015-04-12T21:25:46Z","updated_at":"2015-04-12T21:25:46Z","body":"I think we have here 2 options how to resolve this now:\r\n\r\n 1. change signature of async callback and modify its signature do the source of callback can recover if the task was interrupted. This will require I think modification of `scalaz.concurrent.Task`, but imho is better and more reliable solution than option (2). My proposed async signature will be : \r\n\r\n```scala \r\n\r\n   Task.async[A] { cb: (Throwable \\ / A) => Boolean) => \r\n.....\r\n        if (cb(a)) // task was not interrupted\r\n        else // task was interrupted, recover \r\n   }\r\n\r\nThat will in fact guarantee, that once task is interrupted, `A` must be handled to not leak any resources. If we would implement this, then with very small changes to runAsync we can have desired exactly once behaviour for cleanups. \r\n\r\n2. Introduce change to await : \r\n\r\n```scala\r\n\r\n  Await(req:F[A],rcv: Throwable \\/ A => Trampoline[Process[F,O]], cleanup: A => Trampoline[Process[F,Nothing]]) \r\n\r\n```\r\n\r\nas previously discussed. \r\n\r\nI am not sure how the cleanup will work in different combinators (pipe, tee, wye) but I hope it is implementable. \r\n\r\nThen on the append associativity, I hope we perhaps can do:\r\n\r\n```scala\r\n\r\ncase class Step[+F[_], +O](head: EmitOrAwait[F, O], next: Cont[F, O]) extends HaltOrStep[F, O] \r\n\r\n case class Cont[+F[_], +O](stack: Vector[Option[Cause] => Trampoline[Process[F, O]]] @uncheckedVariance) \r\n\r\n```\r\n\r\nwhere append will be defined by passing None, whereas repeat, onHalt etc. will stay as they are. \r\nPerhaps (and I am not sure of this) this may also made the counter in step redundant. \r\n\r\n"}}