Fix memory leak: do not use the same promise to instanciate all gadgets.

Seems the .then method never realse the memory if the original promise is kept.
Manually create new promise instead each time.

renderjs.js

@@ -1231,15 +1231,27 @@
   }
 
   renderJS.declareGadgetKlass = function declareGadgetKlass(url) {
+    var tmp_constructor,
+      defer;
+
     if (gadget_model_defer_dict.hasOwnProperty(url)) {
       // Return klass object if it already exists
-      return gadget_model_defer_dict[url].promise;
-    }
-
-    var tmp_constructor,
+      if (gadget_model_defer_dict[url].hasOwnProperty('defer_list')) {
+        // Klass not yet loaded.
+        // Add a new defer
         defer = RSVP.defer();
+        gadget_model_defer_dict[url].defer_list.push(defer);
+        return defer.promise;
+      }
+      if (gadget_model_defer_dict[url].is_resolved) {
+        return RSVP.resolve(gadget_model_defer_dict[url].result);
+      }
+      return RSVP.reject(gadget_model_defer_dict[url].result);
+    }
 
-    gadget_model_defer_dict[url] = defer;
+    gadget_model_defer_dict[url] = {
+      defer_list: []
+    };
 
     // Fetch the HTML page and parse it
     return new RSVP.Queue()
@@ -1269,13 +1281,27 @@
         return RSVP.all(promise_list);
       })
       .push(function handleGadgetKlassLoadingSuccess() {
-        defer.resolve(tmp_constructor);
+        var i,
+          len = gadget_model_defer_dict[url].defer_list.length;
+        for (i = 0; i < len; i += 1) {
+          gadget_model_defer_dict[url].defer_list[i].resolve(tmp_constructor);
+        }
+        delete gadget_model_defer_dict[url].defer_list;
+        gadget_model_defer_dict[url].result = tmp_constructor;
+        gadget_model_defer_dict[url].is_resolved = true;
         return tmp_constructor;
       })
       .push(undefined, function handleGadgetKlassLoadingError(e) {
         // Drop the current loading klass info used by selector
         // even in case of error
-        defer.reject(e);
+        var i,
+          len = gadget_model_defer_dict[url].defer_list.length;
+        for (i = 0; i < len; i += 1) {
+          gadget_model_defer_dict[url].defer_list[i].reject(e);
+        }
+        delete gadget_model_defer_dict[url].defer_list;
+        gadget_model_defer_dict[url].result = e;
+        gadget_model_defer_dict[url].is_resolved = false;
         throw e;
       });
   };

test/renderjs_test.js

@@ -3974,6 +3974,74 @@
       });
   });
 
+  test('One failing gadget can not be reloaded', function () {
+    var gadget = new RenderJSGadget(),
+      html_url = 'https://example.org/files/qunittest/test98709.html',
+      error;
+    gadget.__sub_gadget_dict = {};
+
+    this.server.respondWith("GET", html_url, [404, {
+      "Content-Type": "text/html"
+    }, "raw html"]);
+
+    stop();
+    expect(2);
+
+    gadget.declareGadget(html_url)
+      .fail(function (e) {
+        error = e;
+        ok(true, 'first gadget should fail');
+        return gadget.declareGadget(html_url);
+      })
+      .fail(function (e) {
+        equal(e, error, 'second gadget should fail the same way');
+      })
+      .always(function () {
+        // Check that only one request has been done.
+        start();
+      });
+  });
+
+  test('Load 2 concurrent failing gadgets in parallel', function () {
+    // Check that dependencies are loaded once if 2 gadgets are created
+    var gadget = new RenderJSGadget(),
+      html_url = 'https://example.org/files/qunittest/test9871.html',
+      load1,
+      load2,
+      error,
+      mock;
+    gadget.__sub_gadget_dict = {};
+
+    this.server.respondWith("GET", html_url, [404, {
+      "Content-Type": "text/html"
+    }, "raw html"]);
+
+    mock = sinon.mock(renderJS, "parseGadgetHTMLDocument");
+    mock.expects("parseGadgetHTMLDocument").once().returns({});
+
+    stop();
+    load1 = gadget.declareGadget(html_url);
+    load2 = gadget.declareGadget(html_url);
+
+    expect(2);
+
+    load1
+      .fail(function (e) {
+        error = e;
+        ok(true, 'load1 should fail');
+        return load2;
+      })
+      .fail(function (e) {
+        equal(e, error, 'load2 must fail like load1');
+      })
+      .always(function () {
+        // Check that only one request has been done.
+        start();
+        mock.verify();
+        mock.restore();
+      });
+  });
+
   test('One failing gadget does not prevent the others to load', function () {
     // Check that dependencies are loaded once if 2 gadgets are created
     var gadget = new RenderJSGadget(),

[Tomáš Peterka]

What exactly is the defer_list which is causing the troubles? I don't understand...

[Tomáš Peterka]

P.S. Maybe comments in the code would help