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

Using "<~" binding function with Signal.Observers causes memory leaks. #824

Open
michalmlu opened this issue Mar 23, 2021 · 3 comments
Open

Comments

@michalmlu
Copy link

michalmlu commented Mar 23, 2021

In the current implementation of the binding operator, signal is not terminated if the target object gets deallocated, which can cause memory leaks.

Let's imagine a property that represents the current User object and is exposed from the UserRepository.
The ViewModel listens for any changes in that object and sends an information in the form of a Signal to the ViewController.

ViewModel has the pipe defined and it's output exposed to the View.
let firstName = Signal<String, Never>.pipe()

somewhere in the code I feed the pipe with the following binding
firstName.input <~ userRepository.user.map { $0.firstName }

The above map is executed every time the user object changes even if the Pipe is deallocated.

Problem is caused by the following code in the UnidirectionalBinding.swift file. Even the note suggests that "The binding will automatically terminate when the target is deinitialized".

extension Signal.Observer {
	/// Binds a source to a target, updating the target's value to the latest
	/// value sent by the source.
	///
	/// - note: Only `value` events will be forwarded to the Observer.
	///         The binding will automatically terminate when the target is
	///         deinitialized, or when the source sends a `completed` event.
	///
	/// - parameters:
	///   - target: A target to be bond to.
	///   - source: A source to bind.
	///
	/// - returns: A disposable that can be used to terminate binding before the
	///            deinitialization of the target or the source's `completed`
	///            event.
	@discardableResult
	public static func <~
		<Source: BindingSource>
		(observer: Signal<Value, Error>.Observer, source: Source) -> Disposable?
		where Source.Value == Value
	{
		return source.producer.startWithValues { [weak observer] in
			observer?.send(value: $0)
		}
	}
}
@RuiAAPeres
Copy link
Member

If you manually dispose the binding via:

let disposable = firstName.input <~ userRepository.user.map { $0.firstName }

Does this resolve the memory leak?

@michalmlu
Copy link
Author

michalmlu commented Jul 22, 2021

Disposing it manually is the current workaround that I use. The problem is that as documentation states it should be released automatically.

The binding will automatically terminate when the target is deinitialized, or when the source sends a completed event.

@braker1nine
Copy link
Contributor

Would it make sense to only take that source producer while observer hasn't been deallocated. I.e. something like...

return source.producer.take(while: { [weak observer] _ in
    observer != nil
}).startWithValues { [weak observer] in
    observer?.send(value: $0)
}

Obviously it's not perfect in that it will only terminate when the target deinitializes AND it receives a value. But maybe a slight improvement 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants