Skip to content

Race condition when 2 calls copying files to the same path #144

@Soulike

Description

@Soulike

The test case below can trigger a race condition and cause the copied file corrupted:

const eventEmitter = new EventEmitter();
let finishCount = 0;

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

const destination = path.join(os.tmpdir(), 'bigFile');
const bigFile1 = path.join(__dirname, 'bigFiles', 'bigFile1');
const bigFile2 = path.join(__dirname, 'bigFiles', 'bigFile2');

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);
    }
    eventEmitter.emit('done');
    console.log('done!');
});

// operation B
ncp(bigFile2, destination, function (err)
{
    if (err)
    {
        return console.error(err);
    }
    eventEmitter.emit('done');
    console.log('done!');
});

eventEmitter.on('done', () =>
{
    finishCount++;
    if (finishCount === 2)
    {
        const hash3 = crypto.createHash('sha1');

        const copiedBigFile = destination;
        const copiedFileContext = fs.readFileSync(copiedBigFile);
        const copiedHash = hash3.update(copiedFileContext).digest();

        assert.ok(copiedHash.equals(original1Hash)
            || copiedHash.equals(original2Hash),
            `Copied file is different from any original files
            originalFile1Hash: ${original1Hash.toString('hex')}
            originalFile2Hash: ${original2Hash.toString('hex')}
            copiedFileHash: ${copiedHash.toString('hex')}
            `);
    }
});

The execution result is:

done!
node:assert:400
    throw err;
    ^

AssertionError [ERR_ASSERTION]: Copied file is different from any original files
            originalFile1Hash: b9b855fd78c3e68c7e127dcba3db8111be1eea6c
            originalFile2Hash: 36c07a7f47f5478248695505f6993e2ecb83b3f5
            copiedFileHash: 62736f6530b1903a91113ebe32a2943a2654942e

I think the bug is caused by the codes here:

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

At the beginning, both A and B think that destination does not exist. So both operation A and B will directly create write streams to destination and write through the streams at the same time, which I think is problematic.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions