Fwd: Be careful not to leak with event listeners

Gavin Sharp gavin at gavinsharp.com
Tue Jul 16 18:52:47 UTC 2013


This is good advice for front-end developers in general.

Gavin


---------- Forwarded message ----------
From: Justin Lebar <justin.lebar at gmail.com>
Date: Tue, Jul 16, 2013 at 9:48 AM
Subject: Be careful not to leak with event listeners
To: "<dev-gaia at lists.mozilla.org>" <dev-gaia at lists.mozilla.org>
Cc: Kyle Huey <khuey at mozilla.com>, Andrew McCreight <amccreight at mozilla.com>


Dear all,

We've been working on some B2G memory leaks lately, and so many of the
ones we found have had to do with event listeners, I thought it
warranted an e-mail to the list.

The essential problem is that an event listener registered on DOM node
N lives for as long as N lives.  If N is long-lived (e.g. it's the
window of the system app), then the event listener will also live for
a long time.

A closure keeps alive everything it can possibly touch.  So if you
touch e.g. a DOM node from within an event listener, that node and all
of its ancestors and descendents will stay alive at least until the
event listener dies.

For example, this would cause |iframe| to stay alive for as long as
|window| lives, even if you remove all other pointers to |iframe|

var iframe = ...;
  window.addEventListener('mozbrowserfoo', function(e) {
    iframe.setVisible(false);
  });

because the mozbrowserfoo event listener lives as long as the window
lives, and keeps the iframe alive.  It's particularly bad to leak an
<iframe mozbrowser> because we then leak all the mozbrowser machinery
for that iframe.  A solution in this case might be to register the
event listener on |iframe| itself, since then the event listener lives
only as long as the iframe lives, and therefore doesn't extend the
iframe's lifetime.  Alternatively, you could pull |iframe| off
e.target.

One of the bugs we fixed did this:

var homescreenIframe = ...;
window.addEventListener('something', function(e) {
  document.doSomething();
});

This leaked homescreenIframe, even though the closure did not touch
homescreenIframe.  The work-around in this case was to pull |document
off e.target.ownerDocument.

We believe this may be a bug in the JS engine (bug 894149), and we're
working on a fix.  For now, please be cautious of touching |document|
or |window| from within an event listener: You may end up keeping many
things alive that you don't mean to.

If you have any questions or doubts, feel free to ask me, khuey, or
mccr8.  This stuff very tricky to do right, and unfortunately these
leaks are difficult to notice and fix, so a bit of prevention may go a
long way.

Regards,
-Justin

https://bugzilla.mozilla.org/show_bug.cgi?id=894135
https://bugzilla.mozilla.org/show_bug.cgi?id=894147
https://bugzilla.mozilla.org/show_bug.cgi?id=894081
_______________________________________________
dev-gaia mailing list
dev-gaia at lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-gaia



More information about the firefox-dev mailing list