HTTPClient leaks easily (or, can we have a .close() method?)
The following code leaks memory on both platforms at the rate of the size of the file being downloaded:
var debug = Ti.API.debug;
var file = Titanium.Filesystem.getFile(
Titanium.Filesystem.applicationSupportDirectory + "/tmp.file");
// work around the bug where onload events are sometimes not delivered
var utils = {};
utils.clearXhrTimer = function () {
if (utils.currentXhrTimer) {
clearInterval(utils.currentXhrTimer);
utils.currentXhrTimer = null;
}
};
// a 6MB file to demonstrate the problem
var url = "http://192.168.1.14/tmp/f9a6eaf7e3779c84229d71dc0da4113e.mp3";
function downloadFile() {
var xhr = Titanium.Network.createHTTPClient();
xhr.setTimeout(60000);
xhr.onerror = function(e) {
utils.clearXhrTimer();
alert("Error: " + e.error);
};
xhr.onload = function() {
debug("onload");
try {
utils.clearXhrTimer();
if (xhr.status == 200) {
if (file.exists()) {
file.deleteFile();
}
file.write(this.responseData);
downloadFile();
}
} catch (err) {
alert (err);
}
};
utils.clearXhrTimer();
utils.currentXhrTimer = setInterval(
function () {}, 1000);
xhr.open('GET', url);
xhr.send();
}
downloadFile();
From what I can tell from the profile output, this is because xhr was never being garbage collected, so the ASIHTTPRequest/Android equivalent is never being deallocated. I imagine what's happening here is that the local xhr variable is being captured by the function, so its references are never released.
I was able to work around the problem by storing xhr as a global variable so I can remove the old reference, but this is quite cumbersome, and I imagine it's a trap many users will fall into. Can we have an explicit .close() method on the HTTP request in the future, much like you have for the DB code?
PS: It would be wonderful if xhrs were backed with a temp file instead of memory, or at least had the option to be that way. Even with the leaks fixed, I can't download a file over about 10MB on Android due to the heap size being exceeded. donthorp suggested I use the ondatastream function to do this myself, but it's very inefficient compared to native code.
8 Answers
-
Hi,
I've busted my head against XHR and after reading the posts from HMax and Don I've put 2 and 2 together, so I am posting my solution.
First part I got from HMax: use ONE global HTTP client and onload function! I am saving bunch of pictures from my server to my iOS device. I must be able to access properties of httpClient, which I am accessing through "this". I have not yet implemented status and http header checks.
var xhr = Titanium.Network.createHTTPClient(); xhr.onload = function() { var myPictures1 = Titanium.Filesystem.getFile(Titanium.Filesystem.resourcesDirectory,this.folderName + this.fileName); myPictures1.write(this.responseData); }; xhr.onerror = function() { Ti.API.info(this.responseData); };
After I create global call for httpClient a use the open and send in a timeinterval function. I got this part from Don. When I press the button my application loads 3 pictures from a web server. The names of the pictures are in an array. Because I used "this." in a global declaration I can call properties of httpClient through: xhr.fileName, xhr.folderName
'inline code'
var mySaveFilenames = ['background2.png','background3.png','background4.png']; // these are filenames of dowmloaded files from the server var myServerFileNames = ['view2_wallpaper.png','view3_wallpaper.png','view4_wallpaper.png']; // these are filenames on the server serverUpdateButton.addEventListener('click',function(e) { var k = 0; var timer = setInterval(function() { //everything runs in setInterval, interval is 1 second (the 1000 below) xhr.fileName = mySaveFilenames[k]; // accessing properties of httpClient xhr.folderName = 'backgrounds/'; xhr.open('GET',serverURLTextField.value + '/' + myServerFileNames[k]); // because there are three files to download - url is dynamicaly created // send the data xhr.send(); xhr.setTimeout(500); // we don't want that send lives for to long k++; // each time on the interval tick, we iterate k // this is the way to exit setInterval, otherwise you are stuck in setInterval function if (k == 3) { clearInterval(timer); } },1000); });
Hope, that someone will find this code usefull :)
-
@Caio: it crashes.
@Damien: can you give on further details on how you managed to avoid this?
Amazing how this severe issue has had no reply by the devs. :(
-
Hey there,
I've bumped on the same problem (downloading a lot of rather small files to application data directory) and I found out that creating several XHR objects to manage simultaneous downloads wasn't a good idea. At all. It leaked, and crashed the app.
I found out that instancing a single (and global) XHR object, and passing custom variants to it did the trick :
var downloader = Ti.Network.createHTTPClient({timeout:10000}); downloader.onload = function(){ //Save my file //Handle errors first, this is only a sample code saveFile(this.folderName, this.fileName, this.responseData); } //custom properties you can access using this.xxx //inside you onload event manager downloader.fileName = 'foo.jpg'; downloader.folderName = 'images'; //open & send downloader.open('GET', fileURL); downloader.send();
This way, you don't have to create several XHR objects and onLoad events attached to them.
This looks similar to the classic closure leak in javascript.
FYI, I was able to download more than 1400 files in a row using this workaround.
HMax
-
I am banging my head against the wall about how to download a lot of small files and not to run into issues with threads and memory.
From what I see, it really seems that :1) when u download with the same client, it doesnt change its location property and gets you the same file over and over again.
2) when u download with the new client each file, the application has issues with memory, even when I take the strictest measures to be sure that there is no more than 1 client on no more than 1 thread working at the same time.
I guess, it seems the same issue…
-
The thing seems to get more complicated, because once you create the client, there is no way to get rid of that new thread anymore, and your events and timers fire several times, so you have to use some sort of the semaphore.
I have no other way to explain why my caching timer was getting called several times (it wrote the same message to Ti.API.info several times, and also something was deleting freshly cached files, in an effort to clean cache), and why introducing a semaphore fixed the issue.
-
A couple of things.
1) The code at the top is recursive, it's not surprising that it runs into trouble.
2) If you want to kick off a another download. Use setTimeout from the onload to kick it off.
3) HTTPClient instances are not intended to be reused. We're going to update the documentation to reflect that.
4) If, after that, it's not cleaning up. Please provide a sample that shows the problem. We don't have the resources to recreate every text description of a problem. So not having reproduction code to work with slows down the process considerably.
-
This is a really big problem. How were you able to remove the reference? By setting it to null?
-
Hi there,
In a recent crash log I noticed that there were 10 (!) threads, probably because I am forced to make a large number of API calls to get XML data required to populate the app.
To access the main area it needs to make 5 calls; when the most important section is selected by the user the app needs to make a further 7 calls.
At the end of each API call function I parse the response and store the data in a property list. Then (at the end of the function) I expect any objects created in that function to be destroyed and memory released.
If that isn't the case (as it sounds from what people are saying with regard to the HTTPClient) then thats a critical issue.
rgds,
Chris.