AvianFlu / ncp

Asynchronous recursive file copying with Node.js.
MIT License
681 stars 103 forks source link

ncp 0.6.0 in node-webkit 0.10.2 - cpu usage goes to 100% and never finishes #53

Closed gerwinbrunner closed 10 years ago

gerwinbrunner commented 10 years ago

Hi,

I just created a node-webkit app on osx (10.9.4) and added ncp 0.6.0.

I'm only calling this code form node-webkit via a button click:

    var fromPath = "/Volumes/6148b320-ed75-4d39-ba7e-d55fe7f5eac0/TestApp.app";
    var toPath = "/var/folders/fp/rt5y2x2x7qvghvsrf03jr6jh0000gn/T/x1.TestApp.app";

    console.log("ncp - pre", fromPath, toPath);
    ncp(fromPath,toPath, function(err){
      console.log("copied files", err);
    });

The app goes immediately to 100% cpu usage and never finishes. the console output "copied files" is NOT printed out.

When I switch to version 0.5.1 the coping works fine.

Any ideas on what the issue here may be?

Cheers, Gerwin

gerwinbrunner commented 10 years ago

Any idea?

seanli commented 10 years ago

Getting the same issue.

gerwinbrunner commented 10 years ago

Feeling good to not be the only one :)

AvianFlu commented 10 years ago

Do those files also have regular paths in the filesystem? Does the same issue happen if the copy is done without the special osx paths?

Either way, it needs some profiling to find out where it's getting stuck, and I don't currently have osx available locally. If someone can gist the output of running it with dtruss -f, or a flame graph or something, we can figure out what's going wrong.

gerwinbrunner commented 10 years ago

A regular copy works in the terminal with the folders mentioned above.

What exactly should I run with dtruss -f ? I can't reproduce an error from the command line. Just let me know what you need and I'll send you the data.

ghostoy commented 10 years ago

node-webkit uses node.js version v0.11.13-pre. This fails ncp testing on line 4:

const modern = /^v0\.1\d\.\d+$/.test(process.version); // modern = false

Thus ncp thought the current node is older than 0.10 and use old stream event "close" (line 26) which never get fired actually (line 118).

Simply remove the last "$" from the regular expression will fix the issue. Like this:

const modern = /^v0\.1\d\.\d+/.test(process.version); // modern = true
dogawaf commented 10 years ago

I had the same issue, and #58 fixed it (now included in ncp 1.0.0)