freedomjs / freedom

Embracing a distributed web
http://freedomjs.org
Apache License 2.0
512 stars 53 forks source link

core.xhr seems to leak resources #301

Open dborkan opened 8 years ago

dborkan commented 8 years ago

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: screen shot 2015-12-14 at 4 25 00 pm

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: screen shot 2015-12-14 at 4 16 56 pm

dborkan commented 8 years ago

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: screen shot 2015-12-14 at 5 06 08 pm

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 commented 8 years ago

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

ryscheng commented 8 years ago

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 https://github.com/freedomjs/freedom/issues/301#issuecomment-164795568.

trevj commented 8 years ago

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 commented 8 years ago

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:

trevj commented 8 years ago

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