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

There needs to be a this.destroy #3

Closed
ghost opened this issue Nov 5, 2010 · 5 comments
Closed

There needs to be a this.destroy #3

ghost opened this issue Nov 5, 2010 · 5 comments

Comments

@ghost
Copy link

ghost commented Nov 5, 2010

There has to be a this.destroy() that releases all the functions in a step, otherwise there is a memory leak in the case of any error or in the case that a function wants to exit without completing the steps.

Simple leak case:

step(function(){ return; }, function(){ });

2nd function is leaked. The 1st should be allowed to call this.destroy() before returning.

@alexanderdean
Copy link

This would be great. Without this, I don't see a safe way of halting a Step flow in the case of e.g. a validation error.

@cortesi
Copy link

cortesi commented Apr 28, 2011

I've tried a variety of early termination scenarios (including the one given by cwolves), and I can't trigger a memory leak. If this bug report is accurate it's a fatal shortcoming - any of you chaps have more information or a reproducible error case?

@ghost
Copy link
Author

ghost commented Apr 29, 2011

I realized a little while ago that this isn't nearly as important in Node.js as it is in most browsers since V8 caches identical functions. So the only leak is the object literal that represents the function(s), not the function itself. So if you have 1,000 calls to the same step in this case, you leak 1,000*# of functions instances of {}.

Note that even this can be solved simply by using step.fn, which I didn't realize when I'd originally posted the bug, so I'm not sure if it's worth really paying attention to.

@alexanderdean
Copy link

Is there documentation for step.fn?

@ghost
Copy link
Author

ghost commented May 2, 2011

@alexanderdean - Not that I see, but it's fairly simple:

var steps = step.fn(function(){
    // code...
}, function(){
    // code...
});

steps();

@ghost ghost closed this as completed Aug 5, 2011
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants