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

File.downgradeLock says it's atomic, but uses flock which is not atomic #9836

Open
xackus opened this issue Sep 24, 2021 · 5 comments
Open
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@xackus
Copy link
Contributor

xackus commented Sep 24, 2021

File.downgradeLock says:

Atomically modifies the lock to be in shared mode, without releasing it.

From flock(2) Linux man page:

Converting a lock (shared to exclusive, or vice versa) is not
guaranteed to be atomic: the existing lock is first removed, and
then a new lock is established. Between these two steps, a
pending lock request by another process may be granted, with the
result that the conversion either blocks, or fails if LOCK_NB was
specified. (This is the original BSD behavior, and occurs on
many other implementations.)

downgradeLock is used in src/Cache.zig.

@andrewrk
Copy link
Member

Welp I think you found the cause of #9439

@andrewrk andrewrk added the bug Observed behavior contradicts documented or intended behavior label Sep 24, 2021
@andrewrk andrewrk added this to the 0.9.0 milestone Sep 24, 2021
@andrewrk andrewrk added the standard library This issue involves writing Zig code for the standard library. label Sep 24, 2021
@uhthomas
Copy link

uhthomas commented Nov 24, 2021

Is there an idea of where this is in the priority queue and when this may be looked at? Unfortunately we've had to disable Zig CC in our CI environment as around 80% of all builds were failing because of this. 😔

@andrewrk
Copy link
Member

andrewrk commented Dec 4, 2021

Is there an idea of where this is in the priority queue and when this may be looked at?

The 0.9.0 Milestone

@andrewrk
Copy link
Member

andrewrk commented Dec 8, 2021

I'm still interested in addressing this issue but it's a slightly lower priority since it didn't turn out to be the cause of #9439 after all. In fact, I suspect that some OS's actually do make changing lock types with flock atomic and I plan to look into that a little bit as part of diagnosing this issue. Anyway this is moving to 0.9.1 milestone to stay on track for our release date of Dec 20.

@andrewrk andrewrk modified the milestones: 0.9.0, 0.9.1 Dec 8, 2021
@xackus
Copy link
Contributor Author

xackus commented Dec 8, 2021

In fact, I suspect that some OS's actually do make changing lock types with flock atomic

Maybe downgrading. If upgrading was atomic, deadlocks would happen when two processes want to upgrade.

@andrewrk andrewrk modified the milestones: 0.9.1, 0.10.0 Feb 3, 2022
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Jun 19, 2023
@andrewrk andrewrk modified the milestones: 0.14.0, 0.15.0 Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

No branches or pull requests

3 participants