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

Critical: Step is skipping steps! #22

Closed
ghost opened this issue Oct 7, 2011 · 4 comments
Closed

Critical: Step is skipping steps! #22

ghost opened this issue Oct 7, 2011 · 4 comments

Comments

@ghost
Copy link

ghost commented Oct 7, 2011

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.

@ghost
Copy link
Author

ghost commented Oct 7, 2011

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).

@ghost
Copy link
Author

ghost commented Oct 7, 2011

... 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 :)

@koichik
Copy link

koichik commented Oct 7, 2011

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

@creationix
Copy link
Owner

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

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