Skip to content

Commit

Permalink
Fixes #5
Browse files Browse the repository at this point in the history
Get rid of memory leak if the enumerator is not enumerated
  • Loading branch information
Chris Scribner committed May 12, 2012
1 parent b911c09 commit 3c4b975
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 21 deletions.
10 changes: 6 additions & 4 deletions benchmark/benchmark.js
Expand Up @@ -105,9 +105,7 @@ suite.addBatch({

});

while(enumerator.moveNext()){

}
enumerator.end();
}

this.callback(null, new Date() - start);
Expand Down Expand Up @@ -332,7 +330,11 @@ suite.addBatch({
var num = enumerator.current;
}

self.callback(null, new Date() - start);
var endTime = new Date() - start;
//Give the gc time to run
setTimeout(function(){
self.callback(null, endTime);
}, 3000)
});
},

Expand Down
2 changes: 2 additions & 0 deletions docs/api.md
Expand Up @@ -268,6 +268,8 @@ Returns an Enumerator, which has the method moveNext and a getter named current.
Enumerators may yield results asynchronously as long as the code calling the enumerator is also in an asyncblock. If the calling code is
not in an asyncblock, the enumerator must return synchronously.

**Warning** - If you create the enumerator and do not use it, a memory leak will occur. Make sure you call moveNext at least once, or enumerator.end().

One of my favorite uses of this sort of thing is graph walking code. It allows you to separate the traversal logic from the
business logic.

Expand Down
2 changes: 1 addition & 1 deletion docs/overview.md
Expand Up @@ -148,7 +148,7 @@ asyncblock(function(){
* The most clean and succinct syntax that asyncblock offers for cases when obtaining results from the async call
* Supports series and parallel operations
* Maintains the "this" context when calling the async function
* Source transformation will allow asyncblock to target harmony generators when avaiable in V8, further increasing performance and removing risk associated with fibers
* Source transformation will allow asyncblock to target harmony generators when avaiable in V8, further increasing performance and removing the dependency on fibers

### Cons

Expand Down
10 changes: 9 additions & 1 deletion lib/asyncblock.js
Expand Up @@ -78,7 +78,15 @@ var asyncblock = function(fn, options) {
fiber = Fiber(fiberContents);

if(options.isGenerator){
return enumerator.getEnumerator(flow, fiber);
var enumer = enumerator.getEnumerator(flow, fiber);

enumer.events.once('end', function(){
fn = null;
fiber = null;
flow = null;
});

return enumer;
} else {
fiber.run();
}
Expand Down
39 changes: 24 additions & 15 deletions lib/enumerator.js
@@ -1,9 +1,11 @@
var Fiber = require('fibers');
var Flow = require('./flow.js').Flow;
var EventEmitter = require('events').EventEmitter;

var Enumerator = function(exec){
this.exec = exec;
this.curr = null;
this.events = new EventEmitter();
};

Enumerator.prototype.__defineGetter__('current', function(){
Expand All @@ -14,7 +16,13 @@ var undef = (function(){})();
Enumerator.prototype.moveNext = function(){
this.curr = this.exec();

return this.curr !== undef;
var hasMore = this.curr !== undef;

if(!hasMore){
this.end();
}

return hasMore;
};

Flow.prototype.yield = function(value){
Expand All @@ -33,15 +41,6 @@ exports.getEnumerator = function(flow, fiber){
//THe generator is initially stopped
flow._light = false;

var runFiber = fiber.run.bind(fiber);

var run = function(){
//Fiber is done generating
if(fiber != null){
return runFiber();
}
};

var enumerator = function(){
//Async generator support
if(Fiber.current != null){
Expand All @@ -55,29 +54,39 @@ exports.getEnumerator = function(flow, fiber){
};

flow.on('yield', callback);

//If the generator doesn't end up generating anything, we don't want to wait forever
//Clean up and return control when the generator returns
flow.on('end', callback);

//We need to delay the running of the generator in case it returns results without blocking
process.nextTick(function(){
if(!flow._light){
run();
if(flow && !flow._light){
fiber.run();
}
});

var result = outerFlow.wait(key);

outerFlow = null;
flow.removeListener('end', callback);
flow.removeListener('yield', callback);

return result;
} else {
return run();
return fiber.run();
}
};

enumerator.__proto__ = new Enumerator(enumerator);
enumerator.end = function(){
enumerator.events.emit('end');

try { fiber.reset(); } catch(e) {}

enumerator = null;
run = null;
fiber = null;
flow = null;
};

return enumerator;
};

0 comments on commit 3c4b975

Please sign in to comment.