Circular reference memory leak?

Question

I am in doubt if the following design pattern would cause a memory leak.
I have been using it for some time with success, but I haven't seen this pattern used by others, so I'd like some confirmation if you see something wrong with it.
As from next month I have to start working on a large project, and I want to know for sure that I can use this without problems, or if I should use another strategy.

controller.js:

var Controller = function(options){ 
}; 

Controller.prototype.makeView = function(options){ 
    options.controller = this; 
    options.otheroption = options.otheroption; 
    var view = new View(options); 
}; 

Controller.prototype.getModel = function(options){ 
    //--- Get model --- 
    var model = new Model(); 
    var promise = model.fetch(); 
    return promise; 
}); 

view.js:

var View = Backbone.View.extend({ 
    initialize: function(options){
        this.controller = options.controller; 
        this.otheroption = options.otheroption; 
    }, 
    getModel: function(){ 
        var promise = this.controller.getModel(); 
        promise.done(_.bind(function(model){
            //Do something with the returned model instance 
        }, this)); 
    }; 
}); 

Instantiate controller, eg. from the router, or another controller:

//--- Instantiate the controller and build the view ---// 
var controller = new Controller(); 
controller.makeView(options)

To me, this doesn't look like a circular reference, because both the controller and view are declared as a local variable. Yet the instantiated view can access the controller functions, which allows me to isolate the RESTful server interactions via models / collections that the view uses.

For me it would seem as if the only reference remaining would be the view that keeps a reference to the controller object. What I do afterwards is clean up the view (I destroy the instance and its references when I don't need it anymore.

Your opinion on this pattern is highly appreciated.
My purpose is to isolate creation of views / server interactions in separate controller files: if you see holes in my method and have a better way of doing it, please share.

Thanks.

Answer 1

JavaScript implementations haven't had a problem with circular references for a long time. (IE6 did have a memory leak from circular references if I recall correctly, which wasn't shared by any other major browser from that period.)

Modern JavaScript implementations perform garbage collection through a "mark and sweep" algorithm. First they scan through your web app's entire memory structure starting from the global object, and mark everything they find. Then they sweep through every object stored in memory and garbage collect anything that wasn't marked. As long as there isn't a reference to your object from the global object or any stored function, it can be garbage collected.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Memory_Management#Mark-and-sweep_algorithm

You're probably thinking of a reference counting-based implementation, which does have issues with memory leaks from circular references. In that implementation as long as one object contained a reference to another, that second object can't be garbage collected. That method was once used in web browsers but not anymore.

Nowadays, most memory leaks are from globally-accessible objects you forget to clean up and accidentally retaining data in function closures (a function that creates another function and passes/saves it somewhere). Since the closure's local variables can be accessed by the function created inside of them, they have to be retained as long as that function exists.

So go ahead and add all the circular references you want. Unless you need to target IE6, your code's fine.

Answer 2

Short answer: There is no memory leak problem in the code you have posted. The view holds a reference to the controller, but not vice versa. So as long as the controller lives longer than the view, that reference does not keep your objects from being garbage-collected. I don't see a circular reference anywhere in your code.

Longer answer: The pitfalls would be in the code you haven't posted. In particular, any event handlers in your view must be cleaned up properly, otherwise your views never fade into oblivion. But you have said in your question that you clean up your view, so I guess you are aware of that sort of problem.

Answer 3

What controller doing is here looks like a utility to me. Could have been easily managed by a global level singleton. I see some issues in first glance.

• Code repetition, assuming you would creating separate Controller for different types of Models and Views, makeView and getModel code needs to be repeated for each controller. If you extending from a BaseController, then you need to pass View and Model Class to getModel and makeView functions.
• How do you handle a use-case where you have to use same model in different Views?
• makeView and getModel is designed assuming for each makeView there would be a getModel call, in assumed order

I would rather write a utility function which can create and deploy views for me.

 var deployView = function(view, config){
     //do the view rendering
     view.render();
     view.$el.appendTo(config.el);
 }
 var createView  = function(config) {

     var view;
     var viewType = 'model';

     if (config.collection || config.Collection) {
         viewType = 'collection';
     }

     if (viewType === 'model') {
         if (config.Model) {
             config.model = new config.Model(config.modelAttributes);
             //fetch if needed
         }
     } else {
         if (config.Collection) {
             config.collection = new config.Collection(config.items);
             //fetch if needed
         }
     }

     var filteredConfig = _.omit(config, 'Collection', 'Model', 'View');
     view = new config.View(filteredConfig);
     deployView(view, filteredConfig)
 }