Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List memory leak fix #363

Merged
merged 4 commits into from Apr 19, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 13 additions & 29 deletions model/model.js
Expand Up @@ -1048,16 +1048,9 @@ steal('can/util','can/observe', function( can ) {
* model instance is removed from the store, freeing memory.
*
*/
bind: function(eventName){
if ( ! ignoreHookup.test( eventName )) {
if ( ! this._bindings ) {
this.constructor.store[this.__get(this.constructor.id)] = this;
this._bindings = 0;
}
this._bindings++;
}

return can.Observe.prototype.bind.apply( this, arguments );
_bindsetup: function(){
this.constructor.store[this.__get(this.constructor.id)] = this;
return can.Observe.prototype._bindsetup.apply( this, arguments );
},
/**
* @function unbind
Expand All @@ -1083,14 +1076,9 @@ steal('can/util','can/observe', function( can ) {
*
* @return {model} the model instance.
*/
unbind : function(eventName){
if(!ignoreHookup.test(eventName)) {
this._bindings--;
if(!this._bindings){
delete this.constructor.store[getId(this)];
}
}
return can.Observe.prototype.unbind.apply(this, arguments);
_bindteardown: function(){
delete this.constructor.store[getId(this)];
return can.Observe.prototype._bindteardown.apply( this, arguments );;
},
// Change `id`.
___set: function( prop, val ) {
Expand Down Expand Up @@ -1243,18 +1231,14 @@ steal('can/util','can/observe', function( can ) {
*
*/
var ML = can.Model.List = can.Observe.List({
setup : function(){
can.Observe.List.prototype.setup.apply(this, arguments );
// Send destroy events.
var self = this;
this.bind('change', function(ev, how){
if(/\w+\.destroyed/.test(how)){
var index = self.indexOf(ev.target);
if (index != -1) {
self.splice(index, 1);
}
_changes: function(ev, attr){
can.Observe.List.prototype._changes.apply(this, arguments );
if(/\w+\.destroyed/.test(attr)){
var index = this.indexOf(ev.target);
if (index != -1) {
this.splice(index, 1);
}
})
}
}
})

Expand Down
46 changes: 29 additions & 17 deletions model/model_test.js
Expand Up @@ -679,21 +679,25 @@ test("store instance updates", function(){
updateCount = 0;

can.fixture("GET /guys", function(){
var guys = [{id: 1, updateCount: updateCount, nested: {count: updateCount}}];
updateCount++;
var guys = [{id: 1, updateCount: updateCount, nested: {count: updateCount}}];
updateCount++;
return guys;
});
stop();

Guy.findAll({}, function(guys){
start();
console.log("1st callback, store is empty",can.isEmptyObject(Guy.store))
start();
console.log("binding on item",can.isEmptyObject(Guy.store))
guys[0].bind('updated', function(){});
ok(Guy.store[1], 'instance stored');
equals(Guy.store[1].updateCount, 0, 'updateCount is 0')
equals(Guy.store[1].nested.count, 0, 'nested.count is 0')
equals(Guy.store[1].updateCount, 0, 'updateCount is 0')
equals(Guy.store[1].nested.count, 0, 'nested.count is 0')
})
Guy.findAll({}, function(guys){
equals(Guy.store[1].updateCount, 1, 'updateCount is 1')
equals(Guy.store[1].nested.count, 1, 'nested.count is 1')
console.log("2nd callback")
equals(Guy.store[1].updateCount, 1, 'updateCount is 1')
equals(Guy.store[1].nested.count, 1, 'nested.count is 1')
})

})
Expand Down Expand Up @@ -886,18 +890,25 @@ test("destroying a model impact the right list", function() {
return def;
}
},{});
var list1 = new Person.List([ new Person({ id : 1 }), new Person({ id : 2 }) ]),
list2 = new Organisation.List([ new Organisation({ id : 1 }), new Organisation({ id : 2 }) ]);
var people = new Person.List([ new Person({ id : 1 }), new Person({ id : 2 }) ]),
orgs = new Organisation.List([ new Organisation({ id : 1 }), new Organisation({ id : 2 }) ]);

// you must be bound to the list to get this
people.bind("length",function(){})
orgs.bind("length",function(){})

// set each person to have an organization
list1[0].attr('organisation', list2[0]);
list1[1].attr('organisation', list2[1]);
people[0].attr('organisation', orgs[0]);
people[1].attr('organisation', orgs[1]);

equal( list1.length, 2, "Initial Person.List has length of 2")
equal( list2.length, 2, "Initial Organisation.List has length of 2")
list2[0].destroy();
equal( list1.length, 2, "After destroying list2[0] Person.List has length of 2")
equal( list2.length, 1, "After destroying list2[0] Organisation.List has length of 1")
equal( people.length, 2, "Initial Person.List has length of 2")
equal( orgs.length, 2, "Initial Organisation.List has length of 2");

orgs[0].destroy();

equal( people.length, 2, "After destroying orgs[0] Person.List has length of 2");

equal( orgs.length, 1, "After destroying orgs[0] Organisation.List has length of 1")

});

Expand Down Expand Up @@ -965,7 +976,8 @@ test("calling destroy with unsaved model triggers destroyed event (#181)", funct
newModel = new MyModel(),
list = new MyModel.List(),
deferred;

// you must bind to a list for this feature
list.bind("length",function(){});
list.push(newModel);
equal(list.attr('length'), 1, "List length as expected");

Expand Down
59 changes: 29 additions & 30 deletions observe/compute/compute.js
@@ -1,4 +1,4 @@
steal('can/util', function(can) {
steal('can/util', 'can/util/bind', function(can, bind) {

// returns the
// - observes and attr methods are called by func
Expand Down Expand Up @@ -439,36 +439,35 @@ steal('can/util', function(can) {
value = newValue;
// might need a way to look up new and oldVal
can.Observe.triggerBatch(computed, "change",[newValue, oldValue])

}
/**
* @function bind
* `compute.bind("change", handler(event, newVal, oldVal))`
*/
computed.bind = function(ev, handler){
can.addEvent.apply(computed, arguments);
if( bindings === 0 ){
computeState.bound = true;
// setup live-binding
on.call(this, updater)

}
bindings++;
}
/**
* @function unbind
* `compute.unbind("change", handler)`
*/
computed.unbind = function(ev, handler){
can.removeEvent.apply(computed, arguments);
bindings--;
if( bindings === 0 ){
off.call(this,updater)
computeState.bound = false;
}

};
return computed;

return can.extend(computed,{
_bindsetup: function(){
if( bindings === 0 ){
computeState.bound = true;
// setup live-binding
on.call(this, updater)
}
bindings++;
},
_bindteardown: function(){
bindings--;
if( bindings === 0 ){
off.call(this,updater)
computeState.bound = false;
}
},
/**
* @function bind
* `compute.bind("change", handler(event, newVal, oldVal))`
*/
bind: bind.bind,
/**
* @function unbind
* `compute.unbind("change", handler(event, newVal, oldVal))`
*/
unbind: bind.unbind
});
};
can.compute.binder = computeBinder;
return can.compute;
Expand Down
91 changes: 91 additions & 0 deletions observe/live_binding.html
@@ -0,0 +1,91 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Memory Leak Test</title>
</head>
<body>

<button class="poll">Start Polling</button>
<button class="stop">Stop Polling</button><br/><br/>
<div id='live'></div>


<script id="dummyEJS" type="text/ejs">
<% stats.each(function(s) { %>
Name: <%= s.attr('name') %>: <%= s.attr('homeruns') %> (<%= s.getAverage() %>)<br/>
<% }) %>
</script>

<script src='../../steal/steal.js'></script>
<script type='text/javascript'>
steal('can',function(){

Stat = can.Model({
id: "name",
findAll: function(){
var def = $.Deferred();


setTimeout(function(){
def.resolve([
{
"id": 0,
"name": "Ryan Braun",
"homeruns": 2,
"atbats": 20,
"hits": 12
},
{
"id": 1,
"name": "Troy Tulowitzki",
"homeruns": 4,
"atbats": 20,
"hits": 8
}
])

},100)

return def;

}
}, {
getAverage: function() {
return (this.attr('hits') / this.attr('atbats')).toFixed(3);
}
});
window.counter = 0;

var poll = function () {

// Increment the counter
counter++;

// Fetch stats from server
Stat.findAll().then(function(stats) {


// Grab the stats model for Ryan Braun
var p = Stat.model({name: 'Ryan Braun'});

// Update his number of home runs to a random number
p.attr('homeruns', Math.floor(Math.random() * 60 + 1));
});

// Store the timer ID so we can cancel the polling

window.timeoutId = setTimeout(arguments.callee,200);
}

Stat.findAll().then(function(stats){
$("#live").append(can.view('dummyEJS', {
stats: stats
}))
poll();
})

});
</script>
</body>
</html>