Merge pull request #4316 from node-red/4221-handle-concurrent-write-file

Ensure storage/util.writeFile handles concurrent write attempts

packages/node_modules/@node-red/runtime/lib/storage/localfilesystem/util.js

@@ -71,30 +71,30 @@ function readFile(path,backupPath,emptyResponse,type) {
     });
 }
 
+const writeFileLocks = {}
+
 module.exports = {
     /**
     * Write content to a file using UTF8 encoding.
     * This forces a fsync before completing to ensure
     * the write hits disk.
     */
-    writeFile: function(path,content,backupPath) {
+    writeFile: async function(path,content,backupPath) {
-        var backupPromise;
+        if (!writeFileLocks[path]) {
-        if (backupPath && fs.existsSync(path)) {
+            writeFileLocks[path] = Promise.resolve()
-            backupPromise = fs.copy(path,backupPath);
-        } else {
-            backupPromise = Promise.resolve();
         }
-
+        const result = writeFileLocks[path].then(async () => {
-        const dirname = fspath.dirname(path);
+            var backupPromise;
-        const tempFile = `${path}.$$$`;
+            if (backupPath && fs.existsSync(path)) {
-
+                await fs.copy(path,backupPath);
-        return backupPromise.then(() => {
-            if (backupPath) {
                 log.trace(`utils.writeFile - copied ${path} TO ${backupPath}`)
             }
-            return fs.ensureDir(dirname)
+
-        }).then(() => {
+            const dirname = fspath.dirname(path);
-            return new Promise(function(resolve,reject) {
+            const tempFile = `${path}.$$$`;
+            await fs.ensureDir(dirname)
+
+            await new Promise(function(resolve,reject) {
                 var stream = fs.createWriteStream(tempFile);
                 stream.on('open',function(fd) {
                     stream.write(content,'utf8',function() {
@@ -110,10 +110,11 @@ module.exports = {
                     log.warn(log._("storage.localfilesystem.fsync-fail",{path: tempFile, message: err.toString()}));
                     reject(err);
                 });
-            });
+            })
-        }).then(() => {
+            
             log.trace(`utils.writeFile - written content to ${tempFile}`)
-            return new Promise(function(resolve,reject) {
+            
+            await new Promise(function(resolve,reject) {
                 fs.rename(tempFile,path,err => {
                     if (err) {
                         log.warn(log._("storage.localfilesystem.fsync-fail",{path: path, message: err.toString()}));
@@ -122,8 +123,10 @@ module.exports = {
                     log.trace(`utils.writeFile - renamed ${tempFile} to ${path}`)
                     resolve();
                 })
-            });
+            })
-        });
+        })
+        writeFileLocks[path] = result.catch(() => {})
+        return result
     },
     readFile: readFile,
 

test/unit/@node-red/runtime/lib/storage/localfilesystem/util_spec.js

@@ -14,11 +14,33 @@
  * limitations under the License.
  **/
 
-var should = require("should");
+const should = require("should");
-var NR_TEST_UTILS = require("nr-test-utils");
+const NR_TEST_UTILS = require("nr-test-utils");
-var util = NR_TEST_UTILS.require("@node-red/runtime/lib/storage/localfilesystem/util");
+const util = NR_TEST_UTILS.require("@node-red/runtime/lib/storage/localfilesystem/util");
+const { mkdtemp, readFile } = require('fs/promises');
+const { join } = require('path');
+const { tmpdir } = require('os');
 
 describe('storage/localfilesystem/util', function() {
+    describe('writeFile', function () {
+        it('manages concurrent calls to modify the same file', async function () {
+            const testDirectory = await mkdtemp(join(tmpdir(), 'nr-test-'));
+            const testFile = join(testDirectory, 'foo.txt')
+            const testBackupFile = testFile + '.$$$'
+
+            let counter = 0
+            const promises = [
+                util.writeFile(testFile, `update-${counter++}`, testBackupFile ),
+                util.writeFile(testFile, `update-${counter++}`, testBackupFile ),
+                util.writeFile(testFile, `update-${counter++}`, testBackupFile )
+            ]
+
+            await Promise.all(promises)
+
+            const result = await readFile(testFile, { encoding: 'utf-8' })
+            result.should.equal('update-2')
+        })
+    })
     describe('parseJSON', function() {
         it('returns parsed JSON', function() {
             var result = util.parseJSON('{"a":123}');