Retain cycles in closures

When I'm writing code I like it to be free from any distractions that
aren't really relevant to the problem that I'm trying to solve. One of
these distractions is having to pay a lot of attention to retain cycles. As
my code grows, I start making extensions to simplify my code.

I've created the following helper for DispatchQueues:

  extension DispatchQueue {
    func async<T: AnyObject>(weak arg: T, execute: @escaping (T) -> Void) {
      async { [weak arg] in
        if let argRef = arg { execute(argRef) }
      }
    }
  }

It allows you to do this:

   DispatchQueue.main.async(weak: self) { me in
    me.updateSomePartOfUI()
  }

When functions are passed as a closure, the compiler won't warn about a
possible retain cycle (there is no need to prefix with self). That's why
I've also created helpers for calling instance functions:

    func blockFor<Target: AnyObject>(_ target: Target, method: @escaping
(Target) -> () -> Void) -> () -> Void {
    return { [weak target] in
      if let targetRef = target { method(targetRef)() }
    }
  }

  func blockFor<Target: AnyObject, Args>(_ target: Target, method:
@escaping (Target) -> (Args) -> Void, args: Args) -> () -> Void {
    return { [weak target] in
      if let targetRef = target { method(targetRef)(args) }
    }
  }

Calls look like this:

  class MyClass {
    func start() {
      performAction(completion: blockFor(self, method: MyClass.done))
    }

    func done() {
      ...
    }
  }

When you look at code samples online or when I'm reviewing code of
colleagues this seems a real issue. A lot of people probably aren't aware
of the vast amounts of memory that will never be released (until their apps
start crashing). I see people just adding self. to silence the complier :frowning:

I'm wondering what can be done to make this easier for developers. Maybe
introduce a 'guard' keyword for closures which skips the whole closure if
the instances aren't around anymore. Since guard is a new keyword in this
context it shouldn't break any code?

   DispatchQueue.main.async { [guard self] in
    self.updateSomePartOfUI()
  }

I don't have any ideas yet for a better way to pass functions as closures.

- Yvo

When I'm writing code I like it to be free from any distractions that aren't really relevant to the problem that I'm trying to solve. One of these distractions is having to pay a lot of attention to retain cycles. As my code grows, I start making extensions to simplify my code.

I've created the following helper for DispatchQueues:

  extension DispatchQueue {
    func async<T: AnyObject>(weak arg: T, execute: @escaping (T) -> Void) {
      async { [weak arg] in
        if let argRef = arg { execute(argRef) }
      }
    }
  }

It allows you to do this:

   DispatchQueue.main.async(weak: self) { me in
    me.updateSomePartOfUI()
  }

Closures handed off to dispatch queues will not cause retain cycles.

John.

···

On Aug 30, 2017, at 6:45 PM, Yvo van Beek via swift-evolution <swift-evolution@swift.org> wrote:

When functions are passed as a closure, the compiler won't warn about a possible retain cycle (there is no need to prefix with self). That's why I've also created helpers for calling instance functions:

    func blockFor<Target: AnyObject>(_ target: Target, method: @escaping (Target) -> () -> Void) -> () -> Void {
    return { [weak target] in
      if let targetRef = target { method(targetRef)() }
    }
  }

  func blockFor<Target: AnyObject, Args>(_ target: Target, method: @escaping (Target) -> (Args) -> Void, args: Args) -> () -> Void {
    return { [weak target] in
      if let targetRef = target { method(targetRef)(args) }
    }
  }

Calls look like this:

  class MyClass {
    func start() {
      performAction(completion: blockFor(self, method: MyClass.done))
    }

    func done() {
      ...
    }
  }

When you look at code samples online or when I'm reviewing code of colleagues this seems a real issue. A lot of people probably aren't aware of the vast amounts of memory that will never be released (until their apps start crashing). I see people just adding self. to silence the complier :frowning:

I'm wondering what can be done to make this easier for developers. Maybe introduce a 'guard' keyword for closures which skips the whole closure if the instances aren't around anymore. Since guard is a new keyword in this context it shouldn't break any code?

   DispatchQueue.main.async { [guard self] in
    self.updateSomePartOfUI()
  }

I don't have any ideas yet for a better way to pass functions as closures.

- Yvo
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Just because self is used in a a closure doesn’t mean a retain cycle has been created. I don’t worry about retain cycles usually, but always make sure to run my app through the Memory instrument before release, since that should detect any cycles that are present.
  That said, it would be very nice if Swift had an elegant solution here.

Jon

···

On Aug 30, 2017, at 6:45 PM, Yvo van Beek via swift-evolution <swift-evolution@swift.org> wrote:

When I'm writing code I like it to be free from any distractions that aren't really relevant to the problem that I'm trying to solve. One of these distractions is having to pay a lot of attention to retain cycles. As my code grows, I start making extensions to simplify my code.

I've created the following helper for DispatchQueues:

  extension DispatchQueue {
    func async<T: AnyObject>(weak arg: T, execute: @escaping (T) -> Void) {
      async { [weak arg] in
        if let argRef = arg { execute(argRef) }
      }
    }
  }

It allows you to do this:

   DispatchQueue.main.async(weak: self) { me in
    me.updateSomePartOfUI()
  }

When functions are passed as a closure, the compiler won't warn about a possible retain cycle (there is no need to prefix with self). That's why I've also created helpers for calling instance functions:

    func blockFor<Target: AnyObject>(_ target: Target, method: @escaping (Target) -> () -> Void) -> () -> Void {
    return { [weak target] in
      if let targetRef = target { method(targetRef)() }
    }
  }

  func blockFor<Target: AnyObject, Args>(_ target: Target, method: @escaping (Target) -> (Args) -> Void, args: Args) -> () -> Void {
    return { [weak target] in
      if let targetRef = target { method(targetRef)(args) }
    }
  }

Calls look like this:

  class MyClass {
    func start() {
      performAction(completion: blockFor(self, method: MyClass.done))
    }

    func done() {
      ...
    }
  }

When you look at code samples online or when I'm reviewing code of colleagues this seems a real issue. A lot of people probably aren't aware of the vast amounts of memory that will never be released (until their apps start crashing). I see people just adding self. to silence the complier :frowning:

I'm wondering what can be done to make this easier for developers. Maybe introduce a 'guard' keyword for closures which skips the whole closure if the instances aren't around anymore. Since guard is a new keyword in this context it shouldn't break any code?

   DispatchQueue.main.async { [guard self] in
    self.updateSomePartOfUI()
  }

I don't have any ideas yet for a better way to pass functions as closures.

- Yvo
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Hi John,

I see that I've used DispatchQueue.main in my example which is probably a
poor choice to demonstrate the issue:

  class MyClass {
    let queue = DispatchQueue(label: "MyApp.MyQueue")

    func start() {
      queue.async {
        otherClass.doSomethingThatTakesAWhile()
      }

      ...

      queue.async {
        self.doSomething()
      }
    }
  }

This would create a retain cycle wouldn't it?

- Yvo

···

On Wed, Aug 30, 2017 at 6:48 PM, John McCall <rjmccall@apple.com> wrote:

On Aug 30, 2017, at 6:45 PM, Yvo van Beek via swift-evolution < > swift-evolution@swift.org> wrote:

When I'm writing code I like it to be free from any distractions that
aren't really relevant to the problem that I'm trying to solve. One of
these distractions is having to pay a lot of attention to retain cycles. As
my code grows, I start making extensions to simplify my code.

I've created the following helper for DispatchQueues:

  extension DispatchQueue {
    func async<T: AnyObject>(weak arg: T, execute: @escaping (T) -> Void) {
      async { [weak arg] in
        if let argRef = arg { execute(argRef) }
      }
    }
  }

It allows you to do this:

   DispatchQueue.main.async(weak: self) { me in
    me.updateSomePartOfUI()
  }

Closures handed off to dispatch queues will not cause retain cycles.

John.

When functions are passed as a closure, the compiler won't warn about a
possible retain cycle (there is no need to prefix with self). That's why
I've also created helpers for calling instance functions:

    func blockFor<Target: AnyObject>(_ target: Target, method: @escaping
(Target) -> () -> Void) -> () -> Void {
    return { [weak target] in
      if let targetRef = target { method(targetRef)() }
    }
  }

  func blockFor<Target: AnyObject, Args>(_ target: Target, method:
@escaping (Target) -> (Args) -> Void, args: Args) -> () -> Void {
    return { [weak target] in
      if let targetRef = target { method(targetRef)(args) }
    }
  }

Calls look like this:

  class MyClass {
    func start() {
      performAction(completion: blockFor(self, method: MyClass.done))
    }

    func done() {
      ...
    }
  }

When you look at code samples online or when I'm reviewing code of
colleagues this seems a real issue. A lot of people probably aren't aware
of the vast amounts of memory that will never be released (until their apps
start crashing). I see people just adding self. to silence the complier :frowning:

I'm wondering what can be done to make this easier for developers. Maybe
introduce a 'guard' keyword for closures which skips the whole closure if
the instances aren't around anymore. Since guard is a new keyword in this
context it shouldn't break any code?

   DispatchQueue.main.async { [guard self] in
    self.updateSomePartOfUI()
  }

I don't have any ideas yet for a better way to pass functions as closures.

- Yvo
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Hi John,

I see that I've used DispatchQueue.main in my example which is probably a poor choice to demonstrate the issue:

  class MyClass {
    let queue = DispatchQueue(label: "MyApp.MyQueue")

    func start() {
      queue.async {
        otherClass.doSomethingThatTakesAWhile()
      }

      ...

      queue.async {
        self.doSomething()
      }
    }
  }

This would create a retain cycle wouldn't it?

You're correct that there's a temporary cycle which lasts until the queue executes the closure. However, temporary cycles like this rarely cause memory leaks for the same reason that local variables rarely cause memory leaks: eventually the closure will be executed or the function will return. The only way that a closure on a dispatch queue can cause a memory leak is if it constantly enqueues new closures that recreate the same cycle.

If you're doing a dispatch with a very substantial delay (in the hundreds of milliseconds), it can still be a good idea to use a weak reference just so objects can be collected faster. But it's not strictly necessary just to avoid leaks.

John.

···

On Aug 30, 2017, at 7:02 PM, Yvo van Beek <yvo@yvo.net> wrote:

- Yvo

On Wed, Aug 30, 2017 at 6:48 PM, John McCall <rjmccall@apple.com <mailto:rjmccall@apple.com>> wrote:

On Aug 30, 2017, at 6:45 PM, Yvo van Beek via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

When I'm writing code I like it to be free from any distractions that aren't really relevant to the problem that I'm trying to solve. One of these distractions is having to pay a lot of attention to retain cycles. As my code grows, I start making extensions to simplify my code.

I've created the following helper for DispatchQueues:

  extension DispatchQueue {
    func async<T: AnyObject>(weak arg: T, execute: @escaping (T) -> Void) {
      async { [weak arg] in
        if let argRef = arg { execute(argRef) }
      }
    }
  }

It allows you to do this:

   DispatchQueue.main.async(weak: self) { me in
    me.updateSomePartOfUI()
  }

Closures handed off to dispatch queues will not cause retain cycles.

John.

When functions are passed as a closure, the compiler won't warn about a possible retain cycle (there is no need to prefix with self). That's why I've also created helpers for calling instance functions:

    func blockFor<Target: AnyObject>(_ target: Target, method: @escaping (Target) -> () -> Void) -> () -> Void {
    return { [weak target] in
      if let targetRef = target { method(targetRef)() }
    }
  }

  func blockFor<Target: AnyObject, Args>(_ target: Target, method: @escaping (Target) -> (Args) -> Void, args: Args) -> () -> Void {
    return { [weak target] in
      if let targetRef = target { method(targetRef)(args) }
    }
  }

Calls look like this:

  class MyClass {
    func start() {
      performAction(completion: blockFor(self, method: MyClass.done))
    }

    func done() {
      ...
    }
  }

When you look at code samples online or when I'm reviewing code of colleagues this seems a real issue. A lot of people probably aren't aware of the vast amounts of memory that will never be released (until their apps start crashing). I see people just adding self. to silence the complier :frowning:

I'm wondering what can be done to make this easier for developers. Maybe introduce a 'guard' keyword for closures which skips the whole closure if the instances aren't around anymore. Since guard is a new keyword in this context it shouldn't break any code?

   DispatchQueue.main.async { [guard self] in
    self.updateSomePartOfUI()
  }

I don't have any ideas yet for a better way to pass functions as closures.

- Yvo
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

FWIW I wouldn't be oposed to have [guard self] for closures. These
retain cycles are so common that is one the corners that I would like
to have the language helping. Usually people uses weak because that's
what they have been told, and sometimes they risk to use unowned
without fully understanding the consequences just because is more
convenient. In any case I feel that the guard `self` = self trick is
used quite a lot so it may be worth providing language help for it to
make the default case safe and convenient at the same time.

···

On Thu, Aug 31, 2017 at 12:25 AM, John McCall via swift-evolution <swift-evolution@swift.org> wrote:

On Aug 30, 2017, at 7:02 PM, Yvo van Beek <yvo@yvo.net> wrote:

Hi John,

I see that I've used DispatchQueue.main in my example which is probably a
poor choice to demonstrate the issue:

  class MyClass {
    let queue = DispatchQueue(label: "MyApp.MyQueue")

    func start() {
      queue.async {
        otherClass.doSomethingThatTakesAWhile()
      }

      ...

      queue.async {
        self.doSomething()
      }
    }
  }

This would create a retain cycle wouldn't it?

You're correct that there's a temporary cycle which lasts until the queue
executes the closure. However, temporary cycles like this rarely cause
memory leaks for the same reason that local variables rarely cause memory
leaks: eventually the closure will be executed or the function will return.
The only way that a closure on a dispatch queue can cause a memory leak is
if it constantly enqueues new closures that recreate the same cycle.

If you're doing a dispatch with a very substantial delay (in the hundreds of
milliseconds), it can still be a good idea to use a weak reference just so
objects can be collected faster. But it's not strictly necessary just to
avoid leaks.

John.

- Yvo

On Wed, Aug 30, 2017 at 6:48 PM, John McCall <rjmccall@apple.com> wrote:

On Aug 30, 2017, at 6:45 PM, Yvo van Beek via swift-evolution >> <swift-evolution@swift.org> wrote:

When I'm writing code I like it to be free from any distractions that
aren't really relevant to the problem that I'm trying to solve. One of these
distractions is having to pay a lot of attention to retain cycles. As my
code grows, I start making extensions to simplify my code.

I've created the following helper for DispatchQueues:

  extension DispatchQueue {
    func async<T: AnyObject>(weak arg: T, execute: @escaping (T) -> Void)
{
      async { [weak arg] in
        if let argRef = arg { execute(argRef) }
      }
    }
  }

It allows you to do this:

   DispatchQueue.main.async(weak: self) { me in
    me.updateSomePartOfUI()
  }

Closures handed off to dispatch queues will not cause retain cycles.

John.

When functions are passed as a closure, the compiler won't warn about a
possible retain cycle (there is no need to prefix with self). That's why
I've also created helpers for calling instance functions:

    func blockFor<Target: AnyObject>(_ target: Target, method: @escaping
(Target) -> () -> Void) -> () -> Void {
    return { [weak target] in
      if let targetRef = target { method(targetRef)() }
    }
  }

  func blockFor<Target: AnyObject, Args>(_ target: Target, method:
@escaping (Target) -> (Args) -> Void, args: Args) -> () -> Void {
    return { [weak target] in
      if let targetRef = target { method(targetRef)(args) }
    }
  }

Calls look like this:

  class MyClass {
    func start() {
      performAction(completion: blockFor(self, method: MyClass.done))
    }

    func done() {
      ...
    }
  }

When you look at code samples online or when I'm reviewing code of
colleagues this seems a real issue. A lot of people probably aren't aware of
the vast amounts of memory that will never be released (until their apps
start crashing). I see people just adding self. to silence the complier :frowning:

I'm wondering what can be done to make this easier for developers. Maybe
introduce a 'guard' keyword for closures which skips the whole closure if
the instances aren't around anymore. Since guard is a new keyword in this
context it shouldn't break any code?

   DispatchQueue.main.async { [guard self] in
    self.updateSomePartOfUI()
  }

I don't have any ideas yet for a better way to pass functions as closures.

- Yvo
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

--
Alejandro Martinez
http://alejandromp.com