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

Another race condition when 2 calls copying files to the same path #145

Open
Soulike opened this issue Nov 26, 2021 · 0 comments
Open

Another race condition when 2 calls copying files to the same path #145

Soulike opened this issue Nov 26, 2021 · 0 comments

Comments

@Soulike
Copy link

Soulike commented Nov 26, 2021

Before copying a file, ncp first checks whether the target file exists. If it exists (writable is false), ncp will delete it and start copying.

ncp/lib/ncp.js

Lines 89 to 97 in 6820b0f

isWritable(target, function (writable) {
if (writable) {
return copyFile(file, target);
}
if(clobber) {
rmFile(target, function () {
copyFile(file, target);
});
}

But the deleted file may be the file being copied by another ncp call at the same time, causing strange behavior.

ncp/lib/ncp.js

Lines 113 to 126 in 6820b0f

function copyFile(file, target) {
var readStream = fs.createReadStream(file.name),
writeStream = fs.createWriteStream(target, { mode: file.mode });
readStream.on('error', onError);
writeStream.on('error', onError);
if(transform) {
transform(readStream, writeStream, file);
} else {
writeStream.on('open', function() {
readStream.pipe(writeStream);
});
}

The test case to reproduce the bug is similar to #144 but with a slight difference:

const hash1 = crypto.createHash('sha1');
const hash2 = crypto.createHash('sha1');

const destination = path.join(os.tmpdir(), 'bigFile');
const bigFile1 = '/path/to/file1';
const bigFile2 = '/path/to/file2';

const file1Content = fs.readFileSync(bigFile1);
const original1Hash = hash1.update(file1Content).digest();
const file2Content = fs.readFileSync(bigFile2);
const original2Hash = hash2.update(file2Content).digest();

fs.rmSync(destination, {recursive: true, force: true});

// operation A
ncp(bigFile1, destination, function (err)
{
    if (err)
    {
        return console.error(err);
    }
    const hash = crypto.createHash('sha1');
    const copiedBigFile = destination;
    const copiedFileContext = fs.readFileSync(copiedBigFile);
    const copiedHash = hash.update(copiedFileContext).digest();

    // AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value
    assert.ok(original1Hash.equals(copiedHash));    // WTF, destination is not bigFile1!
});

setTimeout(() =>
{
    ncp(bigFile2, destination, function (err)
    {
        if (err)
        {
            return console.error(err);
        }

        // deal with the bug in #143
        setTimeout(() =>
        {
            const hash = crypto.createHash('sha1');
            const copiedBigFile = destination;
            const copiedFileContext = fs.readFileSync(copiedBigFile);
            const copiedHash = hash.update(copiedFileContext).digest();

            assert.ok(original2Hash.equals(copiedHash));
        }, 1000)
    });
}, 100);  // a small delay here
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

1 participant