AvianFlu / ncp

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

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

Open Soulike opened 2 years ago

Soulike commented 2 years ago

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:

https://github.com/AvianFlu/ncp/blob/6820b0fbe3f7400fdb283c741ef88b6d8e2d4994/lib/ncp.js#L113-L126

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.