From e5c3a9668b9088cc802df1f58bf7efd23d228759 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 13 Feb 2023 17:14:22 +0100 Subject: [PATCH 1/2] lib/utils/fs.go: Do not remove lockfiles on Windows --- lib/utils/fs.go | 8 ++++---- lib/utils/fs_unix.go | 9 --------- lib/utils/fs_windows.go | 25 ++++++++----------------- 3 files changed, 12 insertions(+), 30 deletions(-) diff --git a/lib/utils/fs.go b/lib/utils/fs.go index 7e8f5c942ba2..a23217939a8e 100644 --- a/lib/utils/fs.go +++ b/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 diff --git a/lib/utils/fs_unix.go b/lib/utils/fs_unix.go index 9e8cda49d8d2..4a91e84e7f19 100644 --- a/lib/utils/fs_unix.go +++ b/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() - } -} diff --git a/lib/utils/fs_windows.go b/lib/utils/fs_windows.go index 04fa2bb82418..c44f0728e25a 100644 --- a/lib/utils/fs_windows.go +++ b/lib/utils/fs_windows.go @@ -23,24 +23,15 @@ 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 - } -} From 03de039b09a6e305951a11c734cfc95c66ffb774 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 13 Feb 2023 17:14:25 +0100 Subject: [PATCH 2/2] Remove unused import from fs_windows.go --- lib/utils/fs_windows.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/utils/fs_windows.go b/lib/utils/fs_windows.go index c44f0728e25a..8961a13eec08 100644 --- a/lib/utils/fs_windows.go +++ b/lib/utils/fs_windows.go @@ -19,10 +19,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -import ( - "os" -) - // On Windows we use auxiliary .lock.tmp files to acquire locks, so we can still read/write target // files themselves. //