Widgets.js not releasing references, causing memory leaks?


#1

Hi there, I think there might be memory leaks from closures in widgets.js. I’ve done some profiling using Chrome Dev Tools which seem to point to leaks in the widgets.js code; what I’m seeing happens across browsers & platforms.

My application pulls in and displays tweets dynamically. It includes widgets.js in the HTML, retrieves embed snippets for tweets from the OEmbed API, inserts the snippets into the innerHTML of newly-created ‘div’ elements, and calls twttr.widgets.load(new_div). When it hits a configured limit, it removes (and nulls) the oldest tweet from the displayed list every time a new one is loaded. It also recursively nulls & removes the attributes and children (and their attributes) from the oldest tweet before it’s removed.

It all works well except that after a while it starts to eat up a lot of RAM. I tested loading it with lots of tweets by saving out the embed html from a bunch of tweets and just pushing them into the page, and leaving it running. After a while it eats all the RAM it can until the tab crashes. (Chrome, Safari, Firefox, the lot.)

After looking everywhere I could find in my code for leaks, I did a 6hr load/soak test with the twttr.widgets.load() call removed, and although the memory went up for a while, it eventually levelled out at around 60-70MB, and over time I could see it settling into cycles of a period of heap growth followed by GC runs dropping it back 10MB or so every so often. Unlike when the widget loader was being called, when it reliably shot up and up to 1GB and then just died.

So I did some profiling using Chrome Dev Tools and found what look like memory leaks in the widgets.js script. It was impossible to see what was going on from the minified version so I downloaded it, beautified it, and ran again, and I found a bunch of red nodes in detached DOM trees, which were pointed to via live yellow nodes in the widgets.js script.

Two examples are at http://imgur.com/nEG3Xkt and http://imgur.com/47z0x0i

The elements that appear to leak are :

  • HTMLQuoteElement
  • HTMLIFrameElement
  • HTMLDivElement
  • HTMLAnchorElement
  • HTMLInputElement
  • HTMLFormElement
  • HTMLScriptElement

blockquote, iframe, div and a are all in the OEmbed HTML, and according to the Dev Tools are referred to by various functions (at lines 2130, 2564, 2562, 2560, 2558, 2556, 2566, 2553, 2550, 2531, 2530, 2529, 2528, 2527, 2524, 2521, 2496, 2493, 2474, 2471, 2466, 2460, 2451, 2499, 2487 of widget.js) in tfw/widget/base. Sure enough, at line 2564 (for example) there’s:

y.find = function (e) {
return e && p.byId[e] ? p.byId[e].element : null
}

or at line 2556:

y.afterLoad = function (e) {
d.push(e)
}

or at line 2499:

layout: function (e) {
return v.enqueue(e)
}

Etc. There are a bunch that Dev Tools calls out as holding references without even referring to the outer function’s variables, I guess because the closure creation just holds the context whether it’s accessed or not, e.g. at lines 2527-2530:

minWidth: function () {},
maxWidth: function () {},
minHeight: function () {},
maxHeight: function () {},

There are inner functions in many places in the code, so I’m guessing I’m only coming up against instances of this when it’s in functions I’m calling via twttr.widgets.load().

The input, form and script instances seem to be related to the “rufous-sandbox” element, or at least they’re referred to in inner functions in tfw/util/tracking. I can’t really see further into those latter references as the trace is buried in “native” sections.

Could someone verify this? Is there any way it could get fixed if so?

Thanks,
Igor


#2

Gentle bump - any Twitter folk able to confirm or deny this?

Thanks!


#3

Thanks Igor for the excellent report. We are going to investigate this as soon as we can. I’ll keep you posted.


#4

Great, thanks Ara. Look forward to hearing more - if there’s anything else I can do or provide, please let me know!


#5

Hi there, wonder if anyone’s had a chance to look at this? Even just verifying would be good. Should I raise an issue or just leave it here? I have a project which can’t launch until this problem is resolved so it would help enormously to know where the issue lies, at least, and obviously would be good to get it fixed if it is indeed in widgets.js.

Thanks!


#6

We acknowledge some possible issues in your use case. Reviewing to fix some easy targets over the next week.


#7

Thanks Niall, that’s great! Look forward to hearing what comes out of that.


#8

Hi again, just wondering if there are any updates here? Hopefully some of the fruit was hanging low :slight_smile:

Cheers,
Igor


#9

Hi, just wonder if there’s been any chance to get anywhere with this?

Thanks,
Igor


#10

Hello Igor,

Sorry about the long delay. I finally got some time to work on this and made a few fixes to address memory leaks. Can you give it another shot and let me know how it goes.


#11

Hey Ara, thanks very much for this!

I hope I should be testing it against the live version on //platform.twitter.com/widgets.js, as that’s what I’m doing. I haven’t had any time to do any proper Dev Tools memory profiling yet, and might not for a little while; as soon as I do, I’ll post up the results here.

In the meantime, I just tried it out using the same “soak” test as before, and initial (fairly unscientific!) results are:

  • Unfortunately on Chrome 38.0.2125.101/Mavericks 10.9.5 it did exactly the same - it just kept allocating more and more memory until it started to grind to a halt around 930MB and died at around 1.1GB.

  • Interestingly though, on Safari 7.1 (9537.85.10.17.1) it took a very long time for the tab to crash; it gracefully reloaded when it did, and while its RAM usage ballooned to start off with, it started dropping drastically after a while and even seemed to level out much lower than its peak. I ran around 55,000 tweets through it so far before it crashed; it allocated up to 8.91GB to the “Safari Web Content” process, but at some point around 23,000, it started dropping, down to around 1.38GB at one point, 1.02GB at another, then jumped back up to 2.5GB or so, then dropped down again. Similar sort of allocate/free cycle I guess. It seemed a lot better at handling the test, because even when receiving 10 tweets a second it was pretty responsive, even if it wasn’t drawing the actual tweet content at that speed, just the DOM furniture around it; I was typing this reply into another Safari tab while it was going on, and there was no perceptible slowdown for what I was doing. On stopping the stream it just sat, loaded the content of the last few tweets and waited patiently; on starting it again, it started loading them. When I dropped the speed down to 2 tweets a second, it started drawing more of the content again.

  • Also interestingly, in Firefox 32.0.3 it performed the best, even loading the full tweet content including images a lot more frequently. It kept on growing the memory allocation to around 4.5GB, then also dropped to about 3.7-4GB where it seemed to level out. This leveling happened a lot faster on Firefox, too. Pleasantly surprising - I assumed Firefox would be slower and “crashier”, for some reason, but it beat out Chrome on this (ultra-specific!) test. It did eventually die, choking up suddenly and unexpectedly, and it took the whole browser with it. (I guess that’s because it doesn’t have the same process isolation as the other browsers.) I think it was probably after about 20K tweets, and at the time it was doing 10 tweets a second.

Like I say, I’ll try to do some more detailed testing with the memory profiler to see what’s happening with the variables. Right now it feels like there’s a definite improvement; some of that could be browser updates, or it could (of course :-)) be the work you’ve put in to widgets.js.

Either way though there’s definitely still a huge reluctance in the VMs to let go of the memory, and Chrome seems to either just die or unilaterally decide at 1GB memory usage to kill what it sees as an errant process, long before the machine memory is exhausted or (what I guess is) GC cycles have stabilized. Whether that’s a consequence of a VM design decision, or code in widgets.js or my app, I’m really not clear yet. Perhaps the variables have gone out of scope, and the browser memory management code is doing some watching over time of what it’s using and trying to keep hold of what it thinks it’s going to need, rather than letting it go too soon and having to reallocate. It still seems to hang on to enough memory for long enough that it reliably dies on Chrome, though.

It’ll be interesting to see how this pans out with a much slower, “normal” rate of tweets being streamed to it. I’ll leave it running the non-soak version overnight on all three browsers and see where it gets to, and then like I say I’ll look into the Dev Tools to see if I can see what’s happening.

Thanks very much for looking into this and doing the work you’ve done, Ara, I’ll report back when I have more info! If you have any initial thoughts on what I’ve said here I’d be really glad to hear them.

All the best,
Igor


#12

Any updates on this? We’re also experiencing memory leaks using widget.js, it’s clearly not releasing resources.

Is there an example where we could recreate the tweet (CSS and all) without calling widget.js, so we can build the tweets directly and have them look exactly like the official twitter tweets without using your widget?

Thanks!


#13

Thanks for asking, we’ll definitely look into this soon. I need more information on what you are doing. Can you share your code or part of it? If we can create a test case script, I can help you out quicker.


#14