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

Fix windows lock race: Lock exclusive after all try lock errors #2800

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

konstin
Copy link
Member

@konstin konstin commented Apr 3, 2024

We don't know what kind of error the OS gives us on try_lock_exclusive with an already locked file, so we assume all those errors are an already locked file and call lock_exclusive.

For example the windows error:

Os {
    code: 33,
    kind: Uncategorized,
    message: "The process cannot access the file because another process has locked a portion of the file.",
}

Fixes #2767

We don't know what kind of error the OS gives us on `try_lock_exclusive` with an already locked file, so we assume all those errors are an already locked file and call `lock_exclusive`.

For example the windows error:

```
Os {
    code: 33,
    kind: Uncategorized,
    message: "The process cannot access the file because another process has locked a portion of the file.",
}
```

Fixes #2767
@konstin konstin added the bug Something isn't working label Apr 3, 2024
@konstin konstin mentioned this pull request Apr 3, 2024
@konstin konstin merged commit 691ed09 into main Apr 3, 2024
35 checks passed
@konstin konstin deleted the konsti/add-filename-to-lock-error branch April 3, 2024 14:02
warn_user!(
"Waiting to acquire lock for {} (lockfile: {})",
resource,
path.user_display(),
);
file.file().lock_exclusive()?;
file.file().lock_exclusive().map_err(|err| {
// Not an fs_err method, we need to build our own path context
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@konstin can you open an upstream issue to add?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's in scope for either project

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows locking crash
2 participants