From 5aea24f69ab5fd2b6bd4c4f1dfa9fffa9f929302 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Sat, 7 Sep 2019 14:32:25 +0100 Subject: [PATCH] fix: handle concurrent writes on windows Windows returns EPERM errors when trying to rename temp files to files that already exist. In our case a file with a given name will always have the same content so if it's created while we are trying to also create it, we can reasonably assume it's ok to use. If we want to be more thorough we could hash the contents of the new file. --- src/index.js | 28 +++++++++++++++++++++++++++- test/index.spec.js | 25 +++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index d5b1826..8c1355a 100644 --- a/src/index.js +++ b/src/index.js @@ -10,7 +10,7 @@ const setImmediate = require('async/setImmediate') const waterfall = require('async/series') const each = require('async/each') const mkdirp = require('mkdirp') -const writeFile = require('fast-write-atomic') +const writeAtomic = require('fast-write-atomic') const path = require('path') const asyncFilter = require('interface-datastore').utils.asyncFilter @@ -19,6 +19,32 @@ const IDatastore = require('interface-datastore') const Key = IDatastore.Key const Errors = IDatastore.Errors +function writeFile (path, contents, callback) { + writeAtomic(path, contents, (err) => { + if (err) { + if (err.code === 'EPERM' && err.syscall === 'rename') { + // fast-write-atomic writes a file to a temp location before renaming it. + // On Windows, if the final file already exists this error is thrown. + // No such error is thrown on Linux/Mac + // Make sure we can read & write to this file + return fs.access(path, fs.constants.F_OK | fs.constants.W_OK, (err) => { + if (err) { + return callback(err) + } + + // The file was created by another context - this means there were + // attempts to write the same block by two different function calls + return callback() + }) + } + + return callback(err) + } + + callback() + }) +} + /* :: export type FsInputOptions = { createIfMissing?: bool, errorIfExists?: bool, diff --git a/test/index.spec.js b/test/index.spec.js index 9e7e275..2b2d06b 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -153,4 +153,29 @@ describe('FsDatastore', () => { } }) }) + + it('can survive concurrent writes', (done) => { + const dir = utils.tmpdir() + const fstore = new FsStore(dir) + const key = new Key('CIQGFTQ7FSI2COUXWWLOQ45VUM2GUZCGAXLWCTOKKPGTUWPXHBNIVOY') + const value = Buffer.from('Hello world') + + parallel( + new Array(100).fill(0).map(() => { + return (cb) => { + fstore.put(key, value, cb) + } + }), + (err) => { + expect(err).to.not.exist() + + fstore.get(key, (err, res) => { + expect(err).to.not.exist() + expect(res).to.deep.equal(value) + + done() + }) + } + ) + }) })