{"action":"closed","number":2758,"pull_request":{"url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/pulls/2758","id":63468080,"html_url":"https://github.com/ReactiveCocoa/ReactiveCocoa/pull/2758","diff_url":"https://github.com/ReactiveCocoa/ReactiveCocoa/pull/2758.diff","patch_url":"https://github.com/ReactiveCocoa/ReactiveCocoa/pull/2758.patch","issue_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/issues/2758","number":2758,"state":"closed","locked":false,"title":"fix leak on lifted sampleOn","user":{"login":"trifonov-ivan","id":2841612,"avatar_url":"https://avatars.githubusercontent.com/u/2841612?v=3","gravatar_id":"","url":"https://api.github.com/users/trifonov-ivan","html_url":"https://github.com/trifonov-ivan","followers_url":"https://api.github.com/users/trifonov-ivan/followers","following_url":"https://api.github.com/users/trifonov-ivan/following{/other_user}","gists_url":"https://api.github.com/users/trifonov-ivan/gists{/gist_id}","starred_url":"https://api.github.com/users/trifonov-ivan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/trifonov-ivan/subscriptions","organizations_url":"https://api.github.com/users/trifonov-ivan/orgs","repos_url":"https://api.github.com/users/trifonov-ivan/repos","events_url":"https://api.github.com/users/trifonov-ivan/events{/privacy}","received_events_url":"https://api.github.com/users/trifonov-ivan/received_events","type":"User","site_admin":false},"body":"## Issue\r\n\r\nin following code\r\n```swift\r\nSignalProducer<FooObject, NoError>(value: FooObject()).sampleOn(sample).start().dispose()\r\n```\r\n\r\n`FooObject` will live since sampler will not deallocate\r\n\r\n## Reason\r\n\r\nin `Signal.sampleOn` code:\r\n\r\n```swift\r\npublic func sampleOn(sampler: Signal<(), NoError>) -> Signal<Value, Error> {\r\n\t\treturn Signal { observer in\r\n\t\t\tlet state = Atomic(SampleState<Value>())\r\n\t\t\tlet disposable = CompositeDisposable()\r\n\t\t\tdisposable += self.observe { event in\r\n\t\t\t\tswitch event {\r\n\t\t\t\tcase let .Next(value):\r\n\t\t\t\t\tstate.modify { st in\r\n\t\t\t\t\t\tvar mutableSt = st\r\n\t\t\t\t\t\tmutableSt.latestValue = value\r\n\t\t\t\t\t\treturn mutableSt\r\n\t\t\t\t\t}\r\n\t\t\t\tcase let .Failed(error):\r\n\t\t\t\t\tobserver.sendFailed(error)\r\n\t\t\t\tcase .Completed:\r\n\t\t\t\t\tlet oldState = state.modify { st in\r\n\t\t\t\t\t\tvar mutableSt = st\r\n\t\t\t\t\t\tmutableSt.signalCompleted = true\r\n\t\t\t\t\t\treturn mutableSt\r\n\t\t\t\t\t}\t\t\t\r\n\t\t\t\t\tif oldState.samplerCompleted {\r\n\t\t\t\t\t\tobserver.sendCompleted()\r\n\t\t\t\t\t}\r\n\t\t\t\tcase .Interrupted:\r\n\t\t\t\t\tobserver.sendInterrupted()\r\n\t\t\t\t}\r\n\t\t\t}\r\n\t\t\tdisposable += sampler.observe { event in\r\n\t\t\t\tswitch event {\r\n\t\t\t\tcase .Next:\r\n\t\t\t\t\tif let value = state.value.latestValue {\r\n\t\t\t\t\t\tobserver.sendNext(value)\r\n\t\t\t\t\t}\r\n\t\t\t\tcase .Completed:\r\n\t\t\t\t\tlet oldState = state.modify { st in\r\n\t\t\t\t\t\tvar mutableSt = st\r\n\t\t\t\t\t\tmutableSt.samplerCompleted = true\r\n\t\t\t\t\t\treturn mutableSt\r\n\t\t\t\t\t}\t\r\n\t\t\t\t\tif oldState.signalCompleted {\r\n\t\t\t\t\t\tobserver.sendCompleted()\r\n\t\t\t\t\t}\r\n\t\t\t\tcase .Interrupted:\r\n\t\t\t\t\tobserver.sendInterrupted()\r\n\t\t\t\tdefault:\r\n\t\t\t\t\tbreak\r\n\t\t\t\t}\r\n\t\t\t}\r\n\t\t\treturn disposable\r\n\t\t}\r\n\t}\r\n```\r\n\r\nwhen `self` received `Complete` event - it will not react for `Interrupted` event - and in this case `state` will live unchanged until any event from `sampler`.\r\n\r\nLet's looks closer to `lift` implementation from `SignalProducer`\r\n\r\n```swift\r\n\tpublic func lift<U, F, V, G>(transform: Signal<Value, Error> -> Signal<U, F> -> Signal<V, G>) -> Signal<U, F> -> SignalProducer<V, G> {\r\n\t\treturn { otherSignal in\r\n\t\t\treturn SignalProducer { observer, outerDisposable in\r\n\t\t\t\tself.startWithSignal { signal, disposable in\r\n\t\t\t\t\touterDisposable += disposable\r\n\t\t\t\t\touterDisposable += transform(signal)(otherSignal).observe(observer)\r\n\t\t\t\t}\r\n\t\t\t}\r\n\t\t}\r\n\t}\r\n```\r\n\r\nwhat happening with `Signal` produced by `transform(signal)(otherSignal)`?\r\nActually - it captured by it's observer in creation block in `sampleOn`, that captured in `self.observe` and in `sampler.observe` blocks.\r\nThe main problem is that `sample.observe` capture `observer` that capture `signal` that capture `sample.observe` - so even if we dispose `outerDisposable` - sampleOn inner state will not be freed until `sampler.observe` will not receive interruption event.\r\n\r\n## Why fix helps?\r\n\r\n```swift \r\n\tprivate func liftLeft<U, F, V, G>(transform: Signal<Value, Error> -> Signal<U, F> -> Signal<V, G>) -> SignalProducer<U, F> -> SignalProducer<V, G> {\r\n\t\treturn { otherProducer in\r\n\t\t\treturn SignalProducer { observer, outerDisposable in\r\n\t\t\t\totherProducer.startWithSignal { otherSignal, otherDisposable in\r\n\t\t\t\t\touterDisposable.addDisposable(otherDisposable)\r\n\t\t\t\t\t\r\n\t\t\t\t\tself.startWithSignal { signal, disposable in\r\n\t\t\t\t\t\touterDisposable.addDisposable(disposable)\r\n\r\n\t\t\t\t\t\ttransform(signal)(otherSignal).observe(observer)\r\n\t\t\t\t\t}\r\n\t\t\t\t}\r\n\t\t\t}\r\n\t\t}\r\n\t}\r\n```\r\n\r\nin this code `otherSignal` that we pass into `sampleOn` as `sampler` just created on start of pipeline - and destroyed on dispose in `startWithSignal` - and that is destroy second binding in sampleOn, that causing deallocation of `signal` produced by `transform(signal)(otherSignal)` that holding state.\r\n\r\n## For discussion\r\n\r\nI think there is a major problem with `lift` abstraction in current implementation.\r\n\r\nFor 'cold' `SignalProducers` every `Signal` within is temporary object, that lifetime totally described by single `Disposable` returning by `SignalProducer` instance.\r\nBut `sampleOn` - is a perfect example when lifted function significantly changes `Signal`'s lifetime and does not provide any way to broke it.\r\nActually, line \r\n```swift\r\nouterDisposable += transform(signal)(otherSignal).observe(observer)\r\n``` \r\nhides that `transform` result could depends on `signal` or `otherSignal` lifetime - and if we dispose it - we only stop observe, but not stop transform.\r\n\r\nAnother implementations of lift wrap `Signals` to `SignalProducers`, that resolves a problem - but i suggest to remove this implementation of lift, cause it potentially buggy.","created_at":"2016-03-19T13:37:23Z","updated_at":"2016-04-26T12:48:47Z","closed_at":"2016-04-26T12:48:47Z","merged_at":null,"merge_commit_sha":"48c031f8a50e3d0e715a89bcd453ed5245a63022","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/pulls/2758/commits","review_comments_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/pulls/2758/comments","review_comment_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/pulls/comments{/number}","comments_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/issues/2758/comments","statuses_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/statuses/9d32ffb208294187dcf798e8b493fbdd74209c1e","head":{"label":"trifonov-ivan:sample-on-leak","ref":"sample-on-leak","sha":"9d32ffb208294187dcf798e8b493fbdd74209c1e","user":{"login":"trifonov-ivan","id":2841612,"avatar_url":"https://avatars.githubusercontent.com/u/2841612?v=3","gravatar_id":"","url":"https://api.github.com/users/trifonov-ivan","html_url":"https://github.com/trifonov-ivan","followers_url":"https://api.github.com/users/trifonov-ivan/followers","following_url":"https://api.github.com/users/trifonov-ivan/following{/other_user}","gists_url":"https://api.github.com/users/trifonov-ivan/gists{/gist_id}","starred_url":"https://api.github.com/users/trifonov-ivan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/trifonov-ivan/subscriptions","organizations_url":"https://api.github.com/users/trifonov-ivan/orgs","repos_url":"https://api.github.com/users/trifonov-ivan/repos","events_url":"https://api.github.com/users/trifonov-ivan/events{/privacy}","received_events_url":"https://api.github.com/users/trifonov-ivan/received_events","type":"User","site_admin":false},"repo":{"id":51523075,"name":"ReactiveCocoa","full_name":"trifonov-ivan/ReactiveCocoa","owner":{"login":"trifonov-ivan","id":2841612,"avatar_url":"https://avatars.githubusercontent.com/u/2841612?v=3","gravatar_id":"","url":"https://api.github.com/users/trifonov-ivan","html_url":"https://github.com/trifonov-ivan","followers_url":"https://api.github.com/users/trifonov-ivan/followers","following_url":"https://api.github.com/users/trifonov-ivan/following{/other_user}","gists_url":"https://api.github.com/users/trifonov-ivan/gists{/gist_id}","starred_url":"https://api.github.com/users/trifonov-ivan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/trifonov-ivan/subscriptions","organizations_url":"https://api.github.com/users/trifonov-ivan/orgs","repos_url":"https://api.github.com/users/trifonov-ivan/repos","events_url":"https://api.github.com/users/trifonov-ivan/events{/privacy}","received_events_url":"https://api.github.com/users/trifonov-ivan/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/trifonov-ivan/ReactiveCocoa","description":"Streams of values over time","fork":true,"url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa","forks_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/forks","keys_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/keys{/key_id}","collaborators_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/teams","hooks_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/hooks","issue_events_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/issues/events{/number}","events_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/events","assignees_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/assignees{/user}","branches_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/branches{/branch}","tags_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/tags","blobs_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/git/refs{/sha}","trees_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/git/trees{/sha}","statuses_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/statuses/{sha}","languages_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/languages","stargazers_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/stargazers","contributors_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/contributors","subscribers_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/subscribers","subscription_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/subscription","commits_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/commits{/sha}","git_commits_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/git/commits{/sha}","comments_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/comments{/number}","issue_comment_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/issues/comments{/number}","contents_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/contents/{+path}","compare_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/compare/{base}...{head}","merges_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/merges","archive_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/downloads","issues_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/issues{/number}","pulls_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/pulls{/number}","milestones_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/milestones{/number}","notifications_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/labels{/name}","releases_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/releases{/id}","deployments_url":"https://api.github.com/repos/trifonov-ivan/ReactiveCocoa/deployments","created_at":"2016-02-11T15:16:26Z","updated_at":"2016-02-11T15:16:29Z","pushed_at":"2016-03-19T12:56:18Z","git_url":"git://github.com/trifonov-ivan/ReactiveCocoa.git","ssh_url":"git@github.com:trifonov-ivan/ReactiveCocoa.git","clone_url":"https://github.com/trifonov-ivan/ReactiveCocoa.git","svn_url":"https://github.com/trifonov-ivan/ReactiveCocoa","homepage":"","size":15421,"stargazers_count":0,"watchers_count":0,"language":"Objective-C","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"ReactiveCocoa:master","ref":"master","sha":"7877f99bdfb4be1c82c4804082e99c35d0a93a91","user":{"login":"ReactiveCocoa","id":3422977,"avatar_url":"https://avatars.githubusercontent.com/u/3422977?v=3","gravatar_id":"","url":"https://api.github.com/users/ReactiveCocoa","html_url":"https://github.com/ReactiveCocoa","followers_url":"https://api.github.com/users/ReactiveCocoa/followers","following_url":"https://api.github.com/users/ReactiveCocoa/following{/other_user}","gists_url":"https://api.github.com/users/ReactiveCocoa/gists{/gist_id}","starred_url":"https://api.github.com/users/ReactiveCocoa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ReactiveCocoa/subscriptions","organizations_url":"https://api.github.com/users/ReactiveCocoa/orgs","repos_url":"https://api.github.com/users/ReactiveCocoa/repos","events_url":"https://api.github.com/users/ReactiveCocoa/events{/privacy}","received_events_url":"https://api.github.com/users/ReactiveCocoa/received_events","type":"Organization","site_admin":false},"repo":{"id":3606624,"name":"ReactiveCocoa","full_name":"ReactiveCocoa/ReactiveCocoa","owner":{"login":"ReactiveCocoa","id":3422977,"avatar_url":"https://avatars.githubusercontent.com/u/3422977?v=3","gravatar_id":"","url":"https://api.github.com/users/ReactiveCocoa","html_url":"https://github.com/ReactiveCocoa","followers_url":"https://api.github.com/users/ReactiveCocoa/followers","following_url":"https://api.github.com/users/ReactiveCocoa/following{/other_user}","gists_url":"https://api.github.com/users/ReactiveCocoa/gists{/gist_id}","starred_url":"https://api.github.com/users/ReactiveCocoa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ReactiveCocoa/subscriptions","organizations_url":"https://api.github.com/users/ReactiveCocoa/orgs","repos_url":"https://api.github.com/users/ReactiveCocoa/repos","events_url":"https://api.github.com/users/ReactiveCocoa/events{/privacy}","received_events_url":"https://api.github.com/users/ReactiveCocoa/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/ReactiveCocoa/ReactiveCocoa","description":"Streams of values over time","fork":false,"url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa","forks_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/forks","keys_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/teams","hooks_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/hooks","issue_events_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/issues/events{/number}","events_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/events","assignees_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/assignees{/user}","branches_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/branches{/branch}","tags_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/tags","blobs_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/git/refs{/sha}","trees_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/statuses/{sha}","languages_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/languages","stargazers_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/stargazers","contributors_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/contributors","subscribers_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/subscribers","subscription_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/subscription","commits_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/commits{/sha}","git_commits_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/git/commits{/sha}","comments_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/comments{/number}","issue_comment_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/issues/comments{/number}","contents_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/contents/{+path}","compare_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/merges","archive_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/downloads","issues_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/issues{/number}","pulls_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/pulls{/number}","milestones_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/milestones{/number}","notifications_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/labels{/name}","releases_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/releases{/id}","deployments_url":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/deployments","created_at":"2012-03-02T22:11:24Z","updated_at":"2016-04-26T10:02:24Z","pushed_at":"2016-04-26T12:48:26Z","git_url":"git://github.com/ReactiveCocoa/ReactiveCocoa.git","ssh_url":"git@github.com:ReactiveCocoa/ReactiveCocoa.git","clone_url":"https://github.com/ReactiveCocoa/ReactiveCocoa.git","svn_url":"https://github.com/ReactiveCocoa/ReactiveCocoa","homepage":"","size":23935,"stargazers_count":13651,"watchers_count":13651,"language":"Objective-C","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":2485,"mirror_url":null,"open_issues_count":108,"forks":2485,"open_issues":108,"watchers":13651,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/pulls/2758"},"html":{"href":"https://github.com/ReactiveCocoa/ReactiveCocoa/pull/2758"},"issue":{"href":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/issues/2758"},"comments":{"href":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/issues/2758/comments"},"review_comments":{"href":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/pulls/2758/comments"},"review_comment":{"href":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/pulls/2758/commits"},"statuses":{"href":"https://api.github.com/repos/ReactiveCocoa/ReactiveCocoa/statuses/9d32ffb208294187dcf798e8b493fbdd74209c1e"}},"merged":false,"mergeable":false,"mergeable_state":"dirty","merged_by":null,"comments":3,"review_comments":0,"commits":1,"additions":39,"deletions":1,"changed_files":2}}