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 atomic write #2

Merged
merged 2 commits into from
Sep 9, 2024
Merged

Fix atomic write #2

merged 2 commits into from
Sep 9, 2024

Conversation

Mic92
Copy link
Contributor

@Mic92 Mic92 commented Sep 5, 2024

No description provided.

write() doesn't guarantee that the data is written to the disk.
@nikstur
Copy link
Owner

nikstur commented Sep 5, 2024

Can you provide a description of what that fixes and how? I fear I don't really get it from the code.

And more concretely: why do I want to always create a new .tmp file instead of just overwriting an existing one?

When two threads override the same file at the same time,
they might end up both opening the same tmp file i.e. `/etc/passwd.tmp`
and than writing their content to the same.

The new approach opens the tmp file with `create_new` flag, which
fails if the file already exists. In that case, it increments the
index and tries again.
@Mic92
Copy link
Contributor Author

Mic92 commented Sep 9, 2024

I updated the commit description:

atomic_rename: fix potential race condition

When two threads override the same file at the same time,
they might end up both opening the same tmp file i.e. `/etc/passwd.tmp`
and than writing their content to the same.

The new approach opens the tmp file with `create_new` flag, which
fails if the file already exists. In that case, it increments the
index and tries again.

@nikstur nikstur merged commit b9f7007 into nikstur:main Sep 9, 2024
1 check passed
@Mic92 Mic92 deleted the fix-atomic-write branch September 10, 2024 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants