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

Race condition in handling of group()-generated callbacks #26

Open
gleber opened this issue Feb 14, 2012 · 4 comments
Open

Race condition in handling of group()-generated callbacks #26

gleber opened this issue Feb 14, 2012 · 4 comments

Comments

@gleber
Copy link

gleber commented Feb 14, 2012

Hello

I believe there is a race condition in handling of group()-generated callbacks in Step. The problem is described in:

http://code.google.com/p/marmalade/issues/detail?id=35

which has been worked around by the following change in Marmalade:

http://code.google.com/r/gleberp-marmalade/source/detail?r=e61d87b61000ba7eaecebdd6482d52057e4e6227

Hope this helps!
Gleb Peregud

@xavi-
Copy link
Contributor

xavi- commented Feb 25, 2012

I'm not sure if nested Step calls are supported, but in any case I'm taking a look at the bug.

@gleber
Copy link
Author

gleber commented Feb 26, 2012

Please do! :)

@ghost
Copy link

ghost commented Aug 18, 2012

I also believe there is a race condition. Reproduction:

var dir = '/a/directory/which/has/subdirectories';
var fs = require('fs');
var step = require('step');

var dirs = function(dir, cb) {
    fs.readdir(dir, function(err, files) {
        step(function() {
            var group = this.group();
            files.forEach(function(file) {
                var cb = group();
                fs.stat(dir + '/' + file, function(err, stats) {
                    if(stats.isDirectory()) {
                        cb(null, dir + '/' + file);
                    } else {
                        cb(null, null);
                    }
                });
            });
        }, function(err, files) {
            var group = this.group();
            files.forEach(function(file) {
                if(file !== null) {
                    dirs(file, group());
                }
            });
        }, cb);
    });
};
var p = null;
var go = function() {
    dirs(dir, function(err, dirs) {
        if(p === null) {
            p = dirs.length;
        }
        if(p !== dirs.length) {
            throw new Error('weird, previous callback had ' + p + ' dirs but now it has ' + dirs.length + ' dirs!');
        }
        go();
    });
};
go();

@raffecat
Copy link

Hi ghost/gleber,

I ran into a similar issue with fs callbacks and step, which I have documented in the comments of this related issue #42.

I have a fork with a fix in place, but I haven't made a clean pull request yet. Even so, if you're blocking on this issue, the proposed work-around is straight-forward.

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

3 participants