lib/utils/fs.go: Do not remove lockfiles on Windows (#21655)

* lib/utils/fs.go: Do not remove lockfiles on Windows

* Remove unused import from fs_windows.go

lib/utils/fs.go

@@ -169,7 +169,7 @@ func FSTryWriteLock(filePath string) (unlock func() error, err error) {
 		return nil, trace.Retry(ErrUnsuccessfulLockTry, "")
 	}
 
-	return unlockWrapper(fileLock.Unlock, fileLock.Path()), nil
+	return fileLock.Unlock, nil
 }
 
 // FSTryWriteLockTimeout tries to grab write lock, it's doing it until locks is acquired, or timeout is expired,
@@ -182,7 +182,7 @@ func FSTryWriteLockTimeout(ctx context.Context, filePath string, timeout time.Du
 		return nil, trace.ConvertSystemError(err)
 	}
 
-	return unlockWrapper(fileLock.Unlock, fileLock.Path()), nil
+	return fileLock.Unlock, nil
 }
 
 // FSTryReadLock tries to grab write lock, returns ErrUnsuccessfulLockTry
@@ -197,7 +197,7 @@ func FSTryReadLock(filePath string) (unlock func() error, err error) {
 		return nil, trace.Retry(ErrUnsuccessfulLockTry, "")
 	}
 
-	return unlockWrapper(fileLock.Unlock, fileLock.Path()), nil
+	return fileLock.Unlock, nil
 }
 
 // FSTryReadLockTimeout tries to grab read lock, it's doing it until locks is acquired, or timeout is expired,
@@ -210,7 +210,7 @@ func FSTryReadLockTimeout(ctx context.Context, filePath string, timeout time.Dur
 		return nil, trace.ConvertSystemError(err)
 	}
 
-	return unlockWrapper(fileLock.Unlock, fileLock.Path()), nil
+	return fileLock.Unlock, nil
 }
 
 // RemoveSecure attempts to securely delete the file by first overwriting the file with random data three times

lib/utils/fs_unix.go

@@ -23,12 +23,3 @@ package utils
 func getPlatformLockFilePath(path string) string {
 	return path
 }
-
-func unlockWrapper(unlockFn func() error, path string) func() error {
-	return func() error {
-		if unlockFn == nil {
-			return nil
-		}
-		return unlockFn()
-	}
-}

lib/utils/fs_windows.go

@@ -19,28 +19,15 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import (
-	"os"
-)
-
-// On Windows we use auxiliary .lock files to acquire locks, so we can still read/write target files
-// themselves. On unlock we delete the .lock file.
-const lockPostfix = ".lock"
+// On Windows we use auxiliary .lock.tmp files to acquire locks, so we can still read/write target
+// files themselves.
+//
+// .lock.tmp files are deliberately not cleaned up. Their presence doesn't matter to the actual
+// locking. Repeatedly removing them on unlock when acquiring dozens of locks in a short timespan
+// was causing flock.Flock.TryRLock to return either "access denied" or "The process cannot access
+// the file because it is being used by another process".
+const lockPostfix = ".lock.tmp"
 
 func getPlatformLockFilePath(path string) string {
 	return path + lockPostfix
 }
-
-func unlockWrapper(unlockFn func() error, path string) func() error {
-	return func() error {
-		if unlockFn == nil {
-			return nil
-		}
-		err := unlockFn()
-
-		// At this point file can be locked again, and we can get an error, so we do our best effort
-		// to remove .lock file, but can't guarantee it. Last locker should be able to successfully clean it.
-		_ = os.Remove(path)
-		return err
-	}
-}