core.xhr seems to leak resources

[dborkan]

It is looking to me like core.xhr might be leaking resources.

Some background: we were noticing if we left uProxy logged into the GitHub social provider, our Chrome app would crash when left running overnight. We found that if we changed our polling interval (which makes several core.xhr requests) to be less frequent we could run longer before the app crashed, or if it was shorter it would crash faster (10 ms polling interval crashed in a few minutes).

We are currently trying XMLHttpRequest instead of core.xhr and it hasn't crashed yet. Also based on the timeline tab in the Chrome debugger, it appears to use less resources.

Here are some graphs from running uProxy in Chrome, logged into GitHub with polling every 100ms.

This graph shows that resource usage keeps increasing when we use core.xhr:

And here is another graph where I replaced the main core.xhr call with XMLHttpRequest (there may be a few other core.xhr calls still in there). You can see it uses far fewer resources (head and listeners) over the same period of time:

[dborkan]

I also made a test Chrome app to try to replicate. This app just makes GET requests to google.com every 10ms.

Here is the graph for using core.xhr:

But when I switched to XMLHttpRequest, for some reason Chrome's timeline doesn't show the memory or listeners.

Also here is the source code I'm using.. There are two versions here, one for core.xhr and one for regular XMLHttpRequest:
freedom-xhr.zip

[trevj]

Great report! We use this a little in uproxy-lib so it's great to have this flagged...

[ryscheng]

Are we tearing down the provider after its no longer needed?

On Tue, Dec 15, 2015, 10:18 trevj notifications@github.com wrote:

Great report! We use this a little in uproxy-lib so it's great to have
this flagged...

—
Reply to this email directly or view it on GitHub
#301 (comment).

[trevj]

Probably not. Almost the only module we actually tear down properly is TCP sockets, in the SOCKS server:
https://github.com/uProxy/uproxy-lib/blob/master/src/net/tcp.ts#L53

[dborkan]

No we haven't been doing .close() for core.xhr, possibly that is the problem.

Here is an example of us doing a core.xhr request: https://github.com/freedomjs/freedom-social-github/blob/1c257f1d752ab02a6190299fadc9bc37227a4e92/src/github-social-provider.js#L226. Some notes / questions on this:

• This code looks almost identical to using a regular XMLHttpRequest, except it's created with freedom['core.xhr'](), and sync calls or property reads are now async (e.g. var status = xhr.status becomes xhr.getStatus().then(function(status) {...}))
• If we were to call something like freedom['core.xhr'].close(xhr) here, would we have to do it in multiple places? In this example I'm thinking immediately before lines 236 and 239, and also in a handler for onerror? Or is there one place where we can close it? Can we still use the xhr object for methods like getStatus after it has been closed?

[trevj]

FYI, it was a pain to get close right for sockets -- think you probably did the right thing here.