{"action":"created","issue":{"url":"https://api.github.com/repos/scalaz/scalaz-stream/issues/326","labels_url":"https://api.github.com/repos/scalaz/scalaz-stream/issues/326/labels{/name}","comments_url":"https://api.github.com/repos/scalaz/scalaz-stream/issues/326/comments","events_url":"https://api.github.com/repos/scalaz/scalaz-stream/issues/326/events","html_url":"https://github.com/scalaz/scalaz-stream/pull/326","id":59998843,"number":326,"title":"Cleaner runAsync without redundant cleanups","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":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":8,"created_at":"2015-03-05T19:02:17Z","updated_at":"2015-03-06T10:32:55Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/scalaz/scalaz-stream/pulls/326","html_url":"https://github.com/scalaz/scalaz-stream/pull/326","diff_url":"https://github.com/scalaz/scalaz-stream/pull/326.diff","patch_url":"https://github.com/scalaz/scalaz-stream/pull/326.patch"},"body":"There's really no reason (that I can think of) for `runAsync` to be implemented using raw `Actor`.  This reimplements it using `Task`, also fixing a bug regarding duplicated `onComplete` evaluation, cleaning up the signaling, making the interrupt mechanism more responsive, and also opens the door to a more reasonable type signature down the road (propagating the `Task` context into `njoin` and `wye`).\r\n\r\nThe first of the two really notable changes here are that we are now using `Task`'s `runAsyncInterruptibly`, which gives us the ability to kill a `Task` (within limits).  This is a strict improvement over the old `Process#runAsync`, which would literally leak threads whenever a `Task` continued running for a long time after the interrupt.  This happens, for example, if we do a `Process.await(p.run) { ... }`, where `p` is some long-running process, running `Process#runAsync` on that result would leak a LOT of computational resources.  No more!\r\n\r\nThe second change is that we are no longer invoking finalizers multiple times.  This is a really subtle issue, but you can see it here:\r\n\r\n```scala\r\n              case Interrupt(cause) if completed.isEmpty =>\r\n                completed = Some(cause)\r\n                Try(cleanup(cause)).run.runAsync(_.fold(\r\n                  rsn0 =>  cb(left(Error(rsn0).causedBy(cause)))\r\n                  , _ => cb(left(cause))\r\n                ))\r\n```\r\n\r\n…and here!\r\n\r\n```scala\r\n              case AwaitDone(r, awt, _) =>\r\n                Try(awt.rcv(EarlyCause(r)).run)\r\n                .kill\r\n                .run.runAsync(_ => ())\r\n```\r\n\r\nCombined with this:\r\n\r\n```scala\r\n              case Step(awt@Await(req, rcv), cont) =>\r\n                req.runAsync(r => a ! AwaitDone(r, awt, cont))\r\n                right((c:EarlyCause) => rcv(left(c)).run +: cont)\r\n```\r\n\r\nSo with the *old* `runAsync`, when we interrupt a process, we immediately run its cleanup, but we don't terminate the task!  Thus, the task will eventually complete… and then run the whole process again.  Hurray!\r\n\r\nThis PR fixes several of the issues that we (@alissapajer and I) have seen in concurrent code, including several out the outstanding \"`wye` issues\"."},"comment":{"url":"https://api.github.com/repos/scalaz/scalaz-stream/issues/comments/77538477","html_url":"https://github.com/scalaz/scalaz-stream/pull/326#issuecomment-77538477","issue_url":"https://api.github.com/repos/scalaz/scalaz-stream/issues/326","id":77538477,"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-03-06T10:32:55Z","updated_at":"2015-03-06T10:32:55Z","body":"@djspiewak great work. \r\n\r\n\r\nBefore merging this I would like little bit to discuss the leakage of await. Because perhaps, this is not issue at all. \r\n\r\nLets see two codes below:\r\n\r\n```scala \r\n\r\nval asyncAllocation = Task.async(???).flatMap { resource => ...  } //bad code\r\nval goodAllocation = await(Task.async(???), resource => .... ) //good code\r\n\r\n```\r\n\r\nI think we usually write resource allocation by good code, and that shall give us opportunity to ALWAYS run necessary cleanups after  asynchronous task. is interrupted (as we will receive Interrupted exception)\r\n\r\nDo you see this same way? \r\n\r\nWe rely in our server code on fact that `onComplete`is always called. And need to be sure that this change won't ever introduce situation when this won't be the case.\r\n\r\n\r\n"}}