Improved workarounds for win32, continued

polyfills.js

@@ -4,6 +4,7 @@ var origCwd = process.cwd
 var cwd = null
 
 var platform = process.env.GRACEFUL_FS_PLATFORM || process.platform
+var win32MaxBackoff = process.env.GRACEFUL_FS_WIN32_MAX_BACKOFF || 5000
 
 process.cwd = function() {
   if (!cwd)
@@ -80,38 +81,120 @@ function patch (fs) {
     fs.lchownSync = function () {}
   }
 
-  // on Windows, A/V software can lock the directory, causing this
-  // to fail with an EACCES or EPERM if the directory contains newly
-  // created files.  Try again on failure, for up to 60 seconds.
-
-  // Set the timeout this long because some Windows Anti-Virus, such as Parity
-  // bit9, may lock files for up to a minute, causing npm package install
-  // failures. Also, take care to yield the scheduler. Windows scheduling gives
-  // CPU to a busy looping process, which can cause the program causing the lock
-  // contention to be starved of CPU by node, so the contention doesn't resolve.
+  // fs.rename and fs.renameSync uses MoveFileEx function on Windows.
+  // MoveFileEx is not atomic and honors Windows sharing modes, compared to
+  // os/syscall.rename used by Linux and OSX that are atomic and does not
+  // care if the file or directory is locked.
+  //
+  // This means that whenever a file or parent directory is locked (in use)
+  // on Windows the rename might fail with EACCS or EPERM errors depending
+  // the sharing mode set on the file and/or directory.
+  // 
+  // These win32-only overrides try to normalize fs.rename/renameSync
+  // behavior so it's more in line with how it works on Linux and OSX.
+  // It does this by retrying a failed rename for up to 5 seconds (or
+  // value of GRACEFUL_FS_WIN32_MAX_BACKOFF) until actually failing.
   if (platform === "win32") {
     fs.rename = (function (fs$rename) { return function (from, to, cb) {
+      try {
+        var stat = fs.statSync(to)
+        if (!stat) return
+        if (stat.isDirectory()) {
+          fs.rmdirSync(to)
+        } else {
+          fs.unlinkSync(to)
+        }
+      } catch (e) {
+        if (e.code === "ENOTEMPTY") {
+          // target directory not empty, can't rename
+          if (cb) cb(e)
+          return
+        }
+        // ignore other errors
+      }
       var start = Date.now()
-      var backoff = 0;
+      var backoff = 0
+      var backoffUntil = start + win32MaxBackoff
       fs$rename(from, to, function CB (er) {
-        if (er
-            && (er.code === "EACCES" || er.code === "EPERM")
-            && Date.now() - start < 60000) {
+        if (er && (er.code === "EACCES" || er.code === "EPERM") && Date.now() < backoffUntil) {
           setTimeout(function() {
-            fs.stat(to, function (stater, st) {
-              if (stater && stater.code === "ENOENT")
-                fs$rename(from, to, CB);
-              else
-                cb(er)
+            fs.stat(from, function (erFrom, statFrom) {
+              fs.stat(to, function (erTo, statTo) {
+                if (erFrom && !erTo) {
+                  // If the source no longer exists we 
+                  // can probably assume it was moved
+                  if (cb) cb(null)
+                } else if (
+                    statFrom && statTo &&
+                    statFrom.size === statTo.size && 
+                    statFrom.ctime === statTo.ctime
+                  ) {
+                  // If the source and target have 
+                  // the same size and ctime, we
+                  // can assume it was moved
+                  if (cb) cb(null)
+                } else
+                  fs$rename(from, to, CB)
+              })
             })
           }, backoff)
-          if (backoff < 100)
-            backoff += 10;
-          return;
+          if (backoff < 250)
+            backoff += 10
+        } else if (backoff && er && er.code === "ENOENT") {
+          // The source does no longer exist so we
+          // can assume it was moved during one of the tries
+          if (cb) cb(null)
+        } else {
+          if (cb) cb(er)
         }
-        if (cb) cb(er)
       })
     }})(fs.rename)
+
+    fs.renameSync = (function (fs$renameSync) { return function (from, to) {
+      try {
+        var stat = fs.statSync(to)
+        if (!stat) return
+        if (stat.isDirectory()) {
+          fs.rmdirSync(to)
+        } else {
+          fs.unlinkSync(to)
+        }
+      } catch (e) {
+        if (e.code === "ENOTEMPTY") {
+          // target directory not empty, can't rename
+          throw e
+        }
+        // ignore other errors
+      }
+      var start = Date.now()
+      var backoff = 0
+      var backoffUntil = start + win32MaxBackoff
+      function tryRename () {
+        try {
+          fs$renameSync(from, to)
+        } catch (e) {
+          if ((e.code === "EACCS" || e.code === "EPERM") && start < backoffUntil) {
+            if (backoff < 100)
+              backoff += 10
+            var waitUntil = Date.now() + backoff
+            while (waitUntil > Date.now()){}
+            tryRename()
+          } else if (backoff > 0 && e.code === "ENOENT") {
+            // The source does no longer exist because so we can
+            // assume it was moved
+          } else {
+            throw e
+          }
+          // Wait until destination exists and source no longer
+          // exists or that we've reached the backoff limit
+          while (
+            (fs.existsSync(from) || !fs.existsSync(to)) &&
+            Date.now() < backoffUntil
+          ) {}
+        }
+      }
+      tryRename()
+    }})(fs.renameSync)
   }
 
   // if read() returns EAGAIN, then just try it again.

test/tmp/test

@@ -0,0 +1 @@
+600

test/windows-rename-polyfill.js

@@ -1,35 +1,130 @@
 process.env.GRACEFUL_FS_PLATFORM = 'win32'
 
 var fs = require('fs')
-fs.rename = function (a, b, cb) {
-  setTimeout(function () {
-    var er = new Error('EPERM blerg')
-    er.code = 'EPERM'
-    cb(er)
-  })
-}
-
 var gfs = require('../')
 var t = require('tap')
-var a = __dirname + '/a'
-var b = __dirname + '/b'
+var tmpDir = __dirname + '/tmp'
+var testFiles = [];
+var id = 0;
 
-t.test('setup', function (t) {
-  try { fs.mkdirSync(a) } catch (e) {}
-  try { fs.mkdirSync(b) } catch (e) {}
+function anyFileExists (files) {
+  var exists = false;
+  for (var i = 0, len = files.length; i < len; i++) {
+      if (fs.existsSync(files[i]))
+        return true
+  }
+  return false
+}
+
+t.test('setup async', function (t) {
+  try { fs.mkdirSync(tmpDir) } catch (e) {}
+  for (var i = 0; i < 500; i++) {
+    var testFile = tmpDir + '/test-' + id++
+    fs.writeFileSync(testFile, id)
+    testFiles.push(testFile)
+  }
   t.end()
 })
 
-t.test('rename', { timeout: 100 }, function (t) {
-  t.plan(1)
+t.test('rename async', { timeout: 5000 }, function (t) {
+  t.plan(testFiles.length * 2)
+  var dest = tmpDir + '/test'
+  testFiles.forEach(function (src) {
+    gfs.rename(src, dest, function (er) {
+      t.error(er, 'Failed to rename file', er)
+      t.notOk(fs.existsSync(src), 'Source file still exists:' + src)
+    })
+  })
+})
+
+t.test('setup sync', function (t) {
+  try { fs.mkdirSync(tmpDir) } catch (e) {}
+  testFiles = []
+  for (var i = 0; i < 100; i++) {
+    var testFile = tmpDir + '/test-' + id++
+    fs.writeFileSync(testFile, id)
+    testFiles.push(testFile)
+  }
+  t.end()
+})
 
-  gfs.rename(a, b, function (er) {
-    t.ok(er)
+t.test('rename sync', { timeout: 5000 }, function (t) {
+  t.plan((testFiles.length * 2) + 1)
+  var done = 0;
+  var errors = 0;
+  var dest = tmpDir + '/test'
+  testFiles.forEach(function (src) {
+    var srcData = fs.readFileSync(src).toString()
+    t.doesNotThrow(function () { gfs.renameSync(src, dest) }, 'Exception thrown when renaming')
+    var destData = fs.readFileSync(dest).toString()
+    t.equal(srcData, destData, 'Data between source and destination differs: ' + srcData + ' !== ' + destData)
   })
+  t.notOk(anyFileExists(testFiles), 'Some source files still exist')
 })
 
 t.test('cleanup', function (t) {
-  try { fs.rmdirSync(a) } catch (e) {}
-  try { fs.rmdirSync(b) } catch (e) {}
+  testFiles.forEach(function (file) {
+    try { fs.removeSync(file) } catch (e) {}
+  })
+  try { fs.removeSync(tmpDir + '/test') } catch (e) {}
+  try { fs.rmdirSync(tmpDir) } catch (e) {}
   t.end()
 })
+
+
+var testDir1 = tmpDir + 'test1'
+var testDir2 = tmpDir + 'test2'
+
+function setupDirs() {
+  try { fs.mkdirSync(testDir1) } catch (e) {}
+  try { fs.mkdirSync(testDir2) } catch (e) {}
+}
+
+function teardownDirs() {
+  try { fs.rmdirSync(testDir1) } catch (e) {}
+  try { fs.rmdirSync(testDir2) } catch (e) {}
+}
+
+t.test('rename async dir to existing', { timeout: 5000 }, function (t) {
+  t.plan(2)
+  setupDirs()
+  gfs.rename(testDir1, testDir2, function (err) {
+    t.notOk(err)
+    if (!err)
+      t.notOk(fs.existsSync(testDir1), 'Source directory still exists')
+    else
+      t.ok(!err)
+    teardownDirs()
+  })
+})
+
+t.test('rename sync dir to existing', { timeout: 5000 }, function (t) {
+  t.plan(2)
+  setupDirs();
+  t.doesNotThrow(function () {
+    gfs.renameSync(testDir1, testDir2)
+  })
+  t.notOk(fs.existsSync(testDir1), 'Source directory still exists')
+  teardownDirs()
+})
+
+t.test('rename async dir to existing, not empty', { timeout: 5000 }, function (t) {
+  t.plan(3)
+  setupDirs()
+  gfs.rename(testDir1, tmpDir, function (err) {
+    t.ok(err)
+    t.equal(err.code, "ENOTEMPTY")
+    t.ok(fs.existsSync(testDir1), 'Source directory was renamed')
+    teardownDirs()
+  })
+})
+
+t.test('rename sync dir to existing, not empty', { timeout: 5000 }, function (t) {
+  t.plan(2)
+  setupDirs();
+  t.throws(function () {
+    gfs.renameSync(testDir1, tmpDir)
+  }, { code: "ENOTEMPTY" });
+  t.ok(fs.existsSync(testDir1), 'Source directory was renamed')
+  teardownDirs()
+})