ES6 Fat Arrow ( => ) functions can cause memory leaks when used with Event Handlers.

leak.js

class Foo {
  
  constructor(element) {
    this.element = element;
    this.setupHandlers();
  }
  
  doFoo() {}
  
  setupHandlers() {
    this.listener = () => {
      this.doFoo();
    }
    this.element.addEventListener('scroll', this.listener, true);
  }
  
  destroy() {
    this.teardownHandlers();
    this.element = null;
  }
  
  teardownHandlers() {
    this.element.removeEventListener('scroll', this.listener, true);
    // without this next line, the `Foo` instance and any other scope present within the instance
    // will leak, preventing garbage collection.
    this.listener = null;
  }
  
}

So a leak would happen here if you called setupHandlers more than once, because you're losing your handle for the previous this.listener, meaning you'll be unable to unregister it withremoveEventListener.

That doesn't have anything to do with arrow functions.

I see the other leak you're talking about, too. Or at least I see how it's possible. It seems like the same thing could happen with any function that closed over an instance variable or a self reference.

@Blesh the setup/teardown issue is guarded against in the actual implementation. Closing over self is the issue.

this.listener = () => {
      this.doFoo();
    }

becomes

var _this2 = this;
this.listener = function() {
      _this2.doFoo();
    }

Which means that the moment that this event handler is executed, additional scopes arise (the scope the handler gets bound to (usually the event) + the scope it was executed in (usually Window)).

I should have noted that it was required for the event to have triggered at least once for the leak to occur.

The reference to the retained object began 11 levels deep, and with a lot of digging I was never able to find a clear path back to something holding it that wasn't in some way circular.

The trouble was, there were hundreds of circular paths between this method and the component that had created the class instance, and nulling the class instance on the component did not break that. Nulling the event handler immediately allowed it to be cleaned up. In this case, 30MB of memory had been retained as a component and it's entire subtree (a big one) had been retained.

I talked with @wycats, and I'm going to do some more digging to try to find the root of what was causing the scope of the handler to retain. He suggested something that had crossed my mind yesterday. Maybe there isn't a reference back, maybe it really is just circular, but it's gotten to a level of complexity that we've actually stumbled on a bug in the garbage collector.

Additionally, this is the exact code involved, and the commit which fixed the issue.
https://github.com/runspired/smoke-and-mirrors/blob/e3de4397ec53746ec935546b1c5a36a3afded814/addon/models/radar.js#L186-L226

Other leaks and other teardown needs were addressed in the commit as well, but all you need to replicate the leak is to remove this line: https://github.com/runspired/smoke-and-mirrors/blob/e3de4397ec53746ec935546b1c5a36a3afded814/addon/models/radar.js#L223

Steps to replicate

• Clone the repo from this commit html-next/smoke-and-mirrors@e3de439
• Remove that line
• set isDevelopingAddon to return true in index.js
• run the dummy app with ember serve
• navigate to http://localhost:4200/#/examples
• take a heap snapshot
• click on infinite scroll from the demos list
• scroll around a bit. The more you scroll the more pronounced the leak will be because more objects will be retained (the vertical-collection was the item being retained.
• navigate back to the demos by clicking the demos link in the navbar
• wait a moment just to ensure that GC has had way more time than necessary to kick in
• take a heap snapshot
• Compare snapshots 1 & 2 by selecting 2
• then in the drop down that says "All Objects" choose "Objects created between snapshot 1 and snapshot 2". - Sort by retained size
• Take a peek at that giant Object, that's the leaked object.

As you navigate around it's references, you'll notice a lot of the paths eventually go back to filterMovement as the closest thing to window (I think 5 levels deep if I recall?), this is the function that's invoked (via backburner, which is executing the loop in a requestAnimationFrame).

Now uncomment out the line that nulls the scroll handler, and repeat. No leak. :/

I guess what I was getting at is it's not necessarily a leak with "arrow functions", specifically. It's more of a leak with that pattern.

Have you tried testing this in Chrome, without transpilation? It natively supports classes and arrow functions in strict mode. I would also recommend trying it isolated from the smoke-and-mirrors project, if you haven't already. From what has been described, I believe this should be reclaimed. ¯_(ツ)_/¯

Unless of course you're holding on to a reference of instance Foo or this.listener elsewhere..clarifying, just in case 😏

Unless of course you're holding on to a reference of instance Foo or this.listener elsewhere..clarifying, just in case

I assumed I was, and spent a huge amount of time digging through the heap and my code making sure things were properly dereferenced. It's still possible, but at this point highly improbable. I will be digging more.

I have not tested without transpilation.