[PATCH] D78897: [Support] raw_fd_ostream can lock file before write

Serge Pavlov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 28 08:02:06 PDT 2020


sepavloff planned changes to this revision.
sepavloff added a comment.

In D78897#2005085 <https://reviews.llvm.org/D78897#2005085>, @labath wrote:

> Overall I feel that this sort of automatic locking will very rarely be the "right tool for the job". I think your follow-up patch sort of demonstrates that, because you've needed to format the output to a temporary string stream in order for it do what you need. I think it might be more reasonable to just leave this out of the raw_ostream class, and do a manual `lockFile` + `write` + `unlockFile` combo where needed.


Of course, this is a matter of convenience. Automatic locking, as implemented in this patch is not flexible enough, it must be improved. But manual operations might be error-prone.

> In fact, if we do that, I'm wondering if locking is really needed, as a write to a `O_APPEND` file is more-or-less guaranteed to be atomic (I think the only problem comes from signals, and I'm not sure if those really happen on local files):
> 
>   O_APPEND
>          The  file is opened in append mode.  Before each write(2), the file offset is positioned
>          at the end of the file, as if with lseek(2).  The modification of the  file  offset  and
>          the write operation are performed as a single atomic step.
>    

Here is a discussion about atomicity of append operation: https://stackoverflow.com/questions/1154446/is-file-append-atomic-in-unix. In short, actually atomicity of seek+write is provided only for data of small size. This statement agree with our experience. Initially we have implementation that relied on atomicity of append operation. Log files were regularly spoiled. Using file locks solved this problem.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78897/new/

https://reviews.llvm.org/D78897





More information about the llvm-commits mailing list