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

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

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

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

Soulike opened this issue Nov 26, 2021 · 0 comments

Comments

@Soulike
Copy link

Soulike commented Nov 26, 2021

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.

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