From a95ac8c1dde53636b2885acea7f3d941ac706d07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Reis?= Date: Mon, 11 Jan 2016 16:40:52 +0000 Subject: [PATCH 1/3] win: serialize read/write/appendFile and rename rename can be used to atomically overwrite files in UNIX, but fails in Windows when the destination is open. A file can be deleted while open but will only be deleted on disk after it is closed, so a new file cannot take its place right away. This change adds a queue, making rename mutually exclusive with read/write/appendFile. --- graceful-fs.js | 59 ++++++++++++++++++++++++++---- polyfills.js | 23 ++++++------ test/concurrentrename.js | 78 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 17 deletions(-) create mode 100644 test/concurrentrename.js diff --git a/graceful-fs.js b/graceful-fs.js index fe3b17c..cda33be 100644 --- a/graceful-fs.js +++ b/graceful-fs.js @@ -24,6 +24,48 @@ if (/\bgfs4\b/i.test(process.env.NODE_DEBUG || '')) { }) } +var fileEnqueue = function (file, elem) { + return elem[1].apply(null, elem[2]) +} +var fileNext = function (file) {} + +if (process.platform === 'win32') { + var fileQueues = {} + + fileEnqueue = function (file, elem) { + var queue = fileQueues[file] + if (!queue) { + fileQueues[file] = [elem[0]] + elem[1].apply(null, elem[2]) + } else if ((queue[queue.length - 1] === 'share') + && (elem[0] === 'share')) { + queue.push('share') + elem[1].apply(null, elem[2]) + } else { + queue.push(elem) + } + } + + fileNext = function (file) { + var queue = fileQueues[file] + var prev = queue.shift() + require('assert').equal(typeof prev, 'string') + if (queue.length === 0) { + delete fileQueues[file] + } else if (queue[0][0] === 'lock') { + var elem = queue[0] + queue[0] = 'lock' + elem[1].apply(null, elem[2]) + } else { + for (var i = 0; (i < queue.length) && (queue[i][0] === 'share'); ++i) { + var elem = queue[i] + queue[i] = 'share' + elem[1].apply(null, elem[2]) + } + } + } +} + module.exports = patch(require('./fs.js')) if (process.env.TEST_GRACEFUL_FS_GLOBAL_PATCH) { module.exports = patch(fs) @@ -53,7 +95,7 @@ fs.closeSync = (function (fs$closeSync) { return function (fd) { function patch (fs) { // Everything that references the open() function needs to be in here - polyfills(fs) + polyfills(fs, fileEnqueue, fileNext) fs.gracefulify = patch fs.FileReadStream = ReadStream; // Legacy name. fs.FileWriteStream = WriteStream; // Legacy name. @@ -68,7 +110,8 @@ function patch (fs) { return go$readFile(path, options, cb) function go$readFile (path, options, cb) { - return fs$readFile(path, options, function (err) { + fileEnqueue(path, [ 'share', fs$readFile, [path, options, function (err) { + fileNext(path) if (err && (err.code === 'EMFILE' || err.code === 'ENFILE')) enqueue([go$readFile, [path, options, cb]]) else { @@ -76,7 +119,7 @@ function patch (fs) { cb.apply(this, arguments) retry() } - }) + }]]) } } @@ -89,7 +132,8 @@ function patch (fs) { return go$writeFile(path, data, options, cb) function go$writeFile (path, data, options, cb) { - return fs$writeFile(path, data, options, function (err) { + fileEnqueue(path, [ 'share', fs$writeFile, [path, data, options, function (err) { + fileNext(path) if (err && (err.code === 'EMFILE' || err.code === 'ENFILE')) enqueue([go$writeFile, [path, data, options, cb]]) else { @@ -97,7 +141,7 @@ function patch (fs) { cb.apply(this, arguments) retry() } - }) + }]]) } } @@ -111,7 +155,8 @@ function patch (fs) { return go$appendFile(path, data, options, cb) function go$appendFile (path, data, options, cb) { - return fs$appendFile(path, data, options, function (err) { + fileEnqueue(path, [ 'share', fs$appendFile, [path, data, options, function (err) { + fileNext(path) if (err && (err.code === 'EMFILE' || err.code === 'ENFILE')) enqueue([go$appendFile, [path, data, options, cb]]) else { @@ -119,7 +164,7 @@ function patch (fs) { cb.apply(this, arguments) retry() } - }) + }]]) } } diff --git a/polyfills.js b/polyfills.js index 5e4f480..1427fd3 100644 --- a/polyfills.js +++ b/polyfills.js @@ -20,7 +20,7 @@ process.chdir = function(d) { module.exports = patch -function patch (fs) { +function patch (fs, fileEnqueue, fileNext) { // (re-)implement some things that are known busted or missing. // lchmod, broken prior to 0.6.2 @@ -75,15 +75,18 @@ function patch (fs) { // created files. Try again on failure, for up to 1 second. if (process.platform === "win32") { fs.rename = (function (fs$rename) { return function (from, to, cb) { - var start = Date.now() - fs$rename(from, to, function CB (er) { - if (er - && (er.code === "EACCES" || er.code === "EPERM") - && Date.now() - start < 1000) { - return fs$rename(from, to, CB) - } - if (cb) cb(er) - }) + fileEnqueue(to, [ 'lock', function (from, to, cb) { + var start = Date.now() + fs$rename(from, to, function CB (er) { + if (er + && (er.code === "EACCES" || er.code === "EPERM") + && Date.now() - start < 1000) { + return fs$rename(from, to, CB) + } + fileNext(to) + if (cb) cb(er) + }) + }, [from, to, cb]]) }})(fs.rename) } diff --git a/test/concurrentrename.js b/test/concurrentrename.js new file mode 100644 index 0000000..ea4ecb4 --- /dev/null +++ b/test/concurrentrename.js @@ -0,0 +1,78 @@ +'use strict' + +var fs = require('../') +var rimraf = require('rimraf') +var mkdirp = require('mkdirp') +var test = require('tap').test +var p = require('path').resolve(__dirname, 'files') + +process.chdir(__dirname) + +// Make sure to reserve the stderr fd +process.stderr.write('') + +var num = 1025 +var paths = new Array(num) + +test('prepare files', function (t) { + rimraf.sync(p) + mkdirp.sync(p) + + t.plan(num + 1) + for (var i = 0; i < num; ++i) { + paths[i] = 'files/file-' + i + fs.writeFile(paths[i], 'content-rename-' + i, 'ascii', function (er) { + if (er) + throw er + t.pass('written') + }) + } + fs.writeFile('files/file', 'initial', 'ascii', function (er) { + if (er) + throw er + t.pass('written') + }) +}) + +test('read and replace files', function (t) { + t.plan(num * 4) + var queue = [] + function CB (er) { + if (er) + throw er + t.pass('renamed') + } + for (var i = 0; i < num; ++i) { + (function (i) { + queue.push(function () { fs.readFile('files/file', 'ascii', CB) }) + queue.push(function () { fs.writeFile('files/file', 'write-' + i, 'ascii', CB) }) + queue.push(function () { fs.appendFile('files/file', 'append-' + i, 'ascii', CB) }) + queue.push(function () { fs.rename(paths[i], 'files/file', CB) }) + })(i) + } + function swap (arr, a, b) { + var tmp = arr[a] + arr[a] = arr[b] + arr[b] = tmp + } + for (var i = queue.length; i >= 2; --i) { + swap(queue, i - 1, Math.floor(Math.random() * i)) + } + for (var i = 0; i < queue.length; ++i) { + queue[i]() + } +}) + +test('confirm renames', function (t) { + t.plan(num) + for (var i = 0; i < num; ++i) { + fs.access(paths[i], function (er) { + t.equal(er.code, 'ENOENT', 'was renamed') + }) + } +}) + +test('cleanup', function (t) { + rimraf.sync(p) + t.end() +}) From fa9e1deea16eb3dbd43e0f9c4834b4858a34819e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Reis?= Date: Mon, 18 Jan 2016 15:27:57 +0000 Subject: [PATCH 2/3] fixup: fallback to fs.exists if fs.access does not exist --- test/concurrentrename.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/concurrentrename.js b/test/concurrentrename.js index ea4ecb4..2d69e10 100644 --- a/test/concurrentrename.js +++ b/test/concurrentrename.js @@ -66,9 +66,15 @@ test('read and replace files', function (t) { test('confirm renames', function (t) { t.plan(num) for (var i = 0; i < num; ++i) { - fs.access(paths[i], function (er) { - t.equal(er.code, 'ENOENT', 'was renamed') - }) + if (fs.access) { + fs.access(paths[i], function (er) { + t.equal(er.code, 'ENOENT', 'was renamed') + }) + } else { + fs.exists(paths[i], function (exists) { + t.notOk(exists, 'was renamed') + }) + } } }) From 16da4e6818088bae142fac5d338413c9bf18db76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Reis?= Date: Tue, 19 Jan 2016 11:29:13 +0000 Subject: [PATCH 3/3] fixup: README update --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index d920aaa..7fc699e 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,9 @@ graceful-fs: On Windows, it retries renaming a file for up to one second if `EACCESS` or `EPERM` error occurs, likely because antivirus software has locked -the directory. +the directory. It also queues up `rename` while `readFile`, `writeFile` +or `appendFile` are running on the target file and vice-versa, to avoid +conflicts replacing a file being accessed by these calls. ## USAGE