Skip to content

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

Merged
ravicious merged 2 commits intomasterfrom
ravicious/windows-lock
Feb 13, 2023
Merged

lib/utils/fs.go: Do not remove lockfiles on Windows#21655
ravicious merged 2 commits intomasterfrom
ravicious/windows-lock

Conversation

@ravicious
Copy link
Copy Markdown
Member

@ravicious ravicious commented Feb 10, 2023

Fixes #21529.

Removing auxiliary lockfiles was added in #16364. Back then we decided to remove lockfiles on unlock because leaving them behind could be confusing for users.

After #19420 was merged, Connect was creating dozens of locks for known_hosts. It turns out that repeatedly deleting known_hosts.lock 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" (see #21529). Getting rid of lockfile cleanup on unlock solves this problem.

This PR also changes the suffix of auxiliary lockfiles from .lock to .lock.tmp to hopefully help users understand that those leftovers don't really affect the execution of tsh, unlike for example git's index.lock.

At the moment, the file lock functions from lib/utils/fs.go are used in:

  • lib/client/trusted_certs_store.go
  • lib/events/filesessions
  • lib/tbot/config

This change will only have an impact on the first location, leaving known_hosts.lock.tmp on Windows, as both teleport and tbot binaries are not available for Windows.

@ravicious
Copy link
Copy Markdown
Member Author

@r0mant @AntonAM Ping as it's urgent, the UX of Connect v12 on Windows will remain super bad until we address this issue.

@ravicious ravicious added this pull request to the merge queue Feb 13, 2023
Merged via the queue into master with commit 4feff21 Feb 13, 2023
@public-teleport-github-review-bot
Copy link
Copy Markdown

@ravicious See the table below for backport results.

Branch Result
branch/v10 Failed
branch/v11 Failed
branch/v12 Failed
branch/v9 Failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Connect on Windows fails with "handshake failed" or "access denied" when fetching resources or syncing cluster

3 participants