creationix/step

Critical: Step is skipping steps!

Opened this issue · 4 comments

Still trying to track down the exact issue, but here's a code snip:

getFileList( dir, dir, cb );
function getFileList( dir, root, cb ){
    var files
      , stats
      , outFiles = [];

    step( function(){
        fs.readdir( dir, this );

    }, function( err, _files ){
        if( err ){ throw err; }

        files = _files;

        var stats = this.group();
        for( var i=0, l=files.length; i<l; i++ ){
            fs.stat( path.join( dir, files[i] ), stats() );
        };

    }, function( err, _stats ){
console.log( 'HERE1' );
console.trace();
    }, function( err, files ){
console.log( 'HERE2' );
console.trace();
        cb( null, files );
    } );
}

from which I get (75% of the time):

HERE1
Trace: 
    at Function.<anonymous> (../docgen/fileList.js:42:9)
    at next (../docgen/node_modules/step/lib/step.js:51:23)
    at check (../docgen/node_modules/step/lib/step.js:73:14)
    at ../docgen/node_modules/step/lib/step.js:86:20
    at check (../docgen/node_modules/step/lib/step.js:101:9)
    at ../docgen/node_modules/step/lib/step.js:118:22
HERE2
Trace: 
    at Function.<anonymous> (../docgen/fileList.js:45:9)
    at next (../docgen/node_modules/step/lib/step.js:51:23)
    at Array.check [as 0] (../docgen/node_modules/step/lib/step.js:73:14)
    at EventEmitter._tickCallback (node.js:126:26)

The other 25% of the time it hangs as expected.

This may be an issue with node -- It appears that the fs.stat callback is getting triggered _before_ the next.parallel() -> process.nextTick() --> check, at least in this specific case (FYI, for some reason I can only get this to happen on the step/test directory. I noticed it because that was a sub-directory of something I was scanning).

... that's exactly what is happening

var fs = require('fs');

fs.stat( '.', function(){ console.log( 'Stat is done' ); });

for( var i=0; i<100000; i++);

process.nextTick( function(){ console.log( 'Next Tick' ); } );

Stat is done shows up first... similarly, the check() pushed to nextTick is getting triggered late and causing a double-execution of next.apply(...). Step is in a race condition with the file-system here :)

I think that this is not a problem of Node.
check() is called more one times than the size of the group.
Probably, at the last twice, pending is 0.
Therefore localCallback() may be called twice.
I think that it needs guard.
workaround:

@@ -95,6 +95,7 @@ function Step() {

     function check() {
       if (pending === 0) {
+        --pending;
         // When group is done, call the callback
         localCallback(error, result);
       }

I don't remember if this was fixed? If this is still an issue please re-open. Also see #24