<div dir="ltr"><div>> The bad news is that there's a new way to leak that you should be aware of.<br><br></div>Why do you say this is a new way to leak? Did this not leak before, and the fix for the |document| leak will introduce it?<br>
<div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jul 16, 2013 at 7:05 PM, Justin Lebar <span dir="ltr"><<a href="mailto:justin.lebar@gmail.com" target="_blank">justin.lebar@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="im">> For now, please be cautious of touching |document|<br>
> or |window| from within an event listener: You may end up keeping many<br>
> things alive that you don't mean to.<br>
<br>
</div>We may have figured this bit out.<br>
<br>
The good news is, we now don't think that you'll cause leaks by<br>
touching |document| or |window| from within a closure.<br>
<br>
The bad news is that there's a new way to leak that you should be aware of.<br>
<br>
Suppose function F contains two functions, f1, and f2, and suppose the<br>
set of variables touched by f1 is C1, and the set of variables touched<br>
by f2 is C2.  Then so long as either f1 or f2 is alive, all variables<br>
in C1 union C2 are kept alive!<br>
<br>
That is to say, all closures within a given function share the same<br>
scope.  As an example:<br>
<br>
function foo() {<br>
  var v1, v2, v3;<br>
<br>
  setTimeout(function f1() {<br>
    dump(v1);<br>
  }, 0);<br>
<br>
  addEventListener("bar", function f2() {<br>
    dump(v2);<br>
  });<br>
}<br>
<br>
f1 and f2 each keep alive /both/ v1 and v2.  So even after the<br>
setTimeout runs, f2 continues to hold alive v2 /and/ v1.<br>
<br>
The scheme under which f1 keeps alive only v1 and f2 keeps alive only<br>
v2 is called a "safe-for-space closure".  It's apparently difficult to<br>
do right, so we shouldn't expect JS engines to do this anytime soon.<br>
See <a href="http://flint.cs.yale.edu/flint/publications/escc.html" target="_blank">http://flint.cs.yale.edu/flint/publications/escc.html</a>.<br>
<br>
Happy hacking,<br>
-Justin<br>
<div class=""><div class="h5"><br>
On Tue, Jul 16, 2013 at 9:48 AM, Justin Lebar <<a href="mailto:justin.lebar@gmail.com">justin.lebar@gmail.com</a>> wrote:<br>
> Dear all,<br>
><br>
> We've been working on some B2G memory leaks lately, and so many of the<br>
> ones we found have had to do with event listeners, I thought it<br>
> warranted an e-mail to the list.<br>
><br>
> The essential problem is that an event listener registered on DOM node<br>
> N lives for as long as N lives.  If N is long-lived (e.g. it's the<br>
> window of the system app), then the event listener will also live for<br>
> a long time.<br>
><br>
> A closure keeps alive everything it can possibly touch.  So if you<br>
> touch e.g. a DOM node from within an event listener, that node and all<br>
> of its ancestors and descendents will stay alive at least until the<br>
> event listener dies.<br>
><br>
> For example, this would cause |iframe| to stay alive for as long as<br>
> |window| lives, even if you remove all other pointers to |iframe|<br>
><br>
> var iframe = ...;<br>
>   window.addEventListener('mozbrowserfoo', function(e) {<br>
>     iframe.setVisible(false);<br>
>   });<br>
><br>
> because the mozbrowserfoo event listener lives as long as the window<br>
> lives, and keeps the iframe alive.  It's particularly bad to leak an<br>
> <iframe mozbrowser> because we then leak all the mozbrowser machinery<br>
> for that iframe.  A solution in this case might be to register the<br>
> event listener on |iframe| itself, since then the event listener lives<br>
> only as long as the iframe lives, and therefore doesn't extend the<br>
> iframe's lifetime.  Alternatively, you could pull |iframe| off<br>
> e.target.<br>
><br>
> One of the bugs we fixed did this:<br>
><br>
> var homescreenIframe = ...;<br>
> window.addEventListener('something', function(e) {<br>
>   document.doSomething();<br>
> });<br>
><br>
> This leaked homescreenIframe, even though the closure did not touch<br>
> homescreenIframe.  The work-around in this case was to pull |document<br>
> off e.target.ownerDocument.<br>
><br>
> We believe this may be a bug in the JS engine (bug 894149), and we're<br>
> working on a fix.  For now, please be cautious of touching |document|<br>
> or |window| from within an event listener: You may end up keeping many<br>
> things alive that you don't mean to.<br>
><br>
> If you have any questions or doubts, feel free to ask me, khuey, or<br>
> mccr8.  This stuff very tricky to do right, and unfortunately these<br>
> leaks are difficult to notice and fix, so a bit of prevention may go a<br>
> long way.<br>
><br>
> Regards,<br>
> -Justin<br>
><br>
> <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=894135" target="_blank">https://bugzilla.mozilla.org/show_bug.cgi?id=894135</a><br>
> <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=894147" target="_blank">https://bugzilla.mozilla.org/show_bug.cgi?id=894147</a><br>
> <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=894081" target="_blank">https://bugzilla.mozilla.org/show_bug.cgi?id=894081</a><br>
_______________________________________________<br>
</div></div><div class=""><div class="h5">firefox-dev mailing list<br>
<a href="mailto:firefox-dev@mozilla.org">firefox-dev@mozilla.org</a><br>
<a href="https://mail.mozilla.org/listinfo/firefox-dev" target="_blank">https://mail.mozilla.org/listinfo/firefox-dev</a><br>
</div></div></blockquote></div><br></div></div></div>