-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
os: document that WriteFile is not atomic #56173
Comments
Trying to think about what other |
as far as I know, open the file in append mode, then call |
@jacobishao that doesn't make sense to me. |
Tbf I don't think this really needs to be documented. If you know how syscalls work it should be obvious that it can't be atomic because it needs an open syscall. |
@yujiri8 the API doesn't prevent the operation from being atomic; see https://pkg.go.dev/github.com/google/renameio#WriteFile. A Go user can read the code behind I don't think there's a need to document whether every OS or I/O API is atomic or not, just like we don't document whether every API is safe for concurrent use, but I think in this case the current documentation is somewhat prone to being misunderstood. |
Updating the docs sounds good to me. |
i would imagine that the docs require a clear heads up just to draw attention, which will be enough to connect the dots. should it discuss/link the alternatives? |
sorry. it's |
How about adding a comment like this: |
@SrijanSriv The docs for |
Change https://go.dev/cl/443495 mentions this issue: |
As I was replying to #56172, I was looking at the docs for https://pkg.go.dev/os#WriteFile, which say:
I know that this operation isn't atomic: for example, if a write to the file fails half-way through, we may be left with a partial file. That is why alternatives like https://pkg.go.dev/github.com/google/renameio#WriteFile exist.
However, by just reading the docs as a new user, it could be an easy misconception. A lot of the other
os
APIs likeCreate
,Mkdir
, orChmod
are atomic at the system level - they only perform a single system call which modifies the filesystem, so we can't be left with partial changes.Perhaps we should add a godoc note that the API is not atomic. That is, that if an error occurs, the file might be left behind. I imagine this won't matter for many cases, but the warning would be very useful for any Go program which cares about invalid or incomplete states.
cc @ianlancetaylor @stapelberg
The text was updated successfully, but these errors were encountered: