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
Bug: Race case in temporary directory creation #6
Comments
Hi @a3957273 , Thanks for letting me know about your issue. May I ask you to provide the driver code you used (e.g. how you implemented parallel exeuction), so that I can reproduce the exact issue you encountered with? Thank you! |
It's quite hard to reliably reproduce, due to the very quick speed of:
I don't have a reliable script to reproduce. But logically, with two separate processes you could run into the following situation:
Due to your use of sync, this is only an issue with our setup where we run multiple Node processes on the same host. It's possible to make this easier to reproduce by adding a |
As far as I understood, this race condition would be correct, if we would have only one I tried to reproduce your issue with some images and I also added some extra info to the logs (see here). As you can see on the timestamps, all of them were running concurrently, without an issue. Here is the driver code I used: const tarballs = [...];
const dtpUploads = tarballs
.map((tarball) => getDTP(tarball))
.map(
(dtp) =>
new Promise((res) => {
dtp.pushToRegistry().then(res);
})
);
(async () => {
await Promise.all(dtpUploads);
})(); |
You have to be running in different processes as all your calls are synchronous, so your above example will actually be running them one at a time (despite them looking to be promises). |
const fs = require('fs')
// run:
// node race.js & node race.js && fg
function synchronousWait(ms) {
let start = Date.now()
let now = start
while (now - start < ms) {
now = Date.now()
}
}
function fake_workDirUtils_extract(name) {
// fake opening a tar file and extracting the metadata.json
fs.writeFileSync('./metadata.json', JSON.stringify({ name }))
}
function fake_workDirUtils_readManifest() {
// make this function take some time
synchronousWait(150)
return JSON.parse(fs.readFileSync('./metadata.json'))
}
function main() {
const name = String(Math.random())
fake_workDirUtils_extract(name)
const metadata = fake_workDirUtils_readManifest()
console.assert(name === metadata.name, JSON.stringify({ name, metadataName: metadata.name }))
console.assert(name !== metadata.name, 'Program saw no race case')
}
main() Save this to a file
And it still succeeds. Run multiple processes at once and you can reliably get a race case:
Observe that one thread didn't see the same file read as it wrote, because we're running multiple simultaneously. This doesn't happen even if you put them all inside promises inside one process because under the hood |
https://github.com/karolyp/docker-tar-pusher/blob/master/src/WorkDirUtils.ts#L10-L1
If two Docker tars are being uploaded at the same time, they use the same folder. To prevent this, generate pseudo-random folder names, e.g:
The text was updated successfully, but these errors were encountered: