Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Feb 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/utils/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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
Expand Down
9 changes: 0 additions & 9 deletions lib/utils/fs_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
29 changes: 8 additions & 21 deletions lib/utils/fs_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}