creationix/step

Race condition in handling of group()-generated callbacks

Opened this issue · 4 comments

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

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

Please do! :)

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();

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.