XMLHttpRequest loop memory leak

Question

I'm trying to check many items against information on ajax URL. But when I'm running this function in browser, memory usage goes above 2 gigs and then browser crashes (Chrome, Firefox). What am I doing wrong? items variable is really big - >200 000 and also includes some large strings.

var items = [1,2,3,4,5,6,7,8,9,10,...,300000]
var activeItems = {}

function loopAjax(){
    for (i=0; i < items.length; i++) {
        var currItem = items[i];
        var request = new XMLHttpRequest();
        var found = 0

        request.open("GET", "/item=" + currItem);
        request.onreadystatechange = function() {
            if (request.readyState == 4 && request.status == 200) {
                var response = JSON.parse(request.responseText);
                var active = response[0].active;
                if (active) {
                    console.log("FOUND ACTIVE! " + currItem);
                    activeItems[found] = {"active": true, "item": currItem};
                    found++;
                }
            }
        }
        request.send();
    }
}

the ...300000 is the problem. You need to redesign how you are approaching this problem — robbmj, Dec 15 '13 at 23:11
That is a horrible design. Think about what you are doing. Making 200,000 requests to the server!? You have no delay so you just force the browser to queue up calls. No break. The server should handle multiple items in a call. Not just one. You need to look into queuing up calls on your end. — epascarello, Dec 15 '13 at 23:12
Woah , that's a hell lot of requests to the server. Why so many requests? — Harsha Venkataramu, Dec 15 '13 at 23:13
Related: [Javascript closure inside loops - simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example). `var`s are *program* or `function`-scoped in JavaScript, not block-scoped. So, you've only declared 1 `request` variable to be shared by all iterations of the `for` loop. And, as Ajax is asynchronous, the value of `request` will change before `onreadystatechange` is called and can verify the state of any one of the `request`s. — Jonathan Lonowski, Dec 15 '13 at 23:14
... which means that the request state/status may never be satisfied. — Qantas 94 Heavy, Dec 15 '13 at 23:15

score 2 · Answer 1 · answered Dec 16 '13 at 02:12

Thank goodness the browser stalls and dies. If it didn't you just created a denial of service attack!

The problem needs to be reapproached. You better off creating a state machine which has a stack of requests in it. That way you only doing say 5 concurrent requests at a time.

function ItemChecker(sample_size, max_threads) {
  this.sample_size = sample_size;
  this.max_threads = max_threads;
  this.counter = 0;
  this.activeItems = [];
  this.isRunning = false;
  this.running_count = 0;
}

ItemChecker.prototype.start = function start() {
  this.isRunning = true;
  while (this.running_count < this.max_threads) {
    this.next();
  }
  return this;
};

ItemChecker.prototype.stop = fucntion stop() {
  this.isRunning = false;
  return this;
};

ItemChecker.prototype.next = function next() {
  var request, item_id, _this = this;

  function xhrFinished(req) {
    var response;
    if (req.readyState !== 4) {
      return;
    }

    _this.counter--;

    if (req.status === 200) {
      try {
        response = JSON.parse(request.responseText);
        if (response[0].active) {
          _this.activeItems.push({
            active: true,
            item: item_id;
          });
        }
      } catch(e) {
        console.error(e);
      }

      // When finished call a callback
      if (_this.onDone && _this.counter >= _this.sample_size) {
        _this.onDone(_this.activeItems);
      }
    }
    else {
      console.warn("Server returned " + req.status);
    }
  }

  if (!this.isRunning || this.counter >= this.sample_size) {
    return;
  }

  item_id = this.counter;
  this.counter++;

  request = new XMLHttpRequest();
  request.onreadystatechange = xhrFinished;
  request.open("GET", "item=" + item_id);
  request.send();
};

ItemChecker.prototype.whenDone = function whenDone(callback) {
  this.onDone = callback;
  return this;
};

This might work? Didn't try it for real. But you would call it with:

var item_checker = new ItemChecker(300000, 5);
item_checker.whenDone(function(active) {
  // Do something with active
}).start();

Of course I would do this for real. I'd do it using promises. — Sukima, Dec 16 '13 at 02:16
Thanks for the answer. I found a way limiting concurrent requests by using setTimeout, but this looks better, I'll try it. It was my own machine anyway :) — Mark, Dec 16 '13 at 03:43
Just to warn you XMLHttpRequests are asynchronous as it is. Adding setTimeouts to the logic will only make the complexity worse. — Sukima, Dec 16 '13 at 03:58

XMLHttpRequest loop memory leak

1 Answers1