[PATCH] D38570: Support: Rewrite Windows implementation of sys::fs::rename to be more POSIXy.

[Zachary Turner via llvm-commits]

But the Source file is just a temporary file right that’s not (yet) part of
the cache right? So why would another process be accessing the same
temporary file instead of creating a different one?


If the problem is with Dest, I could understand, but it opens that one with
sharing so it can’t be that.

Is the Source file already part of the cache for some reason?

Btw, I have an (absolutely terrible) idea how to solve this “for real”, but
I’ll save it until I understand the problem better
On Wed, Oct 4, 2017 at 6:38 PM Peter Collingbourne via Phabricator <
reviews at reviews.llvm.org> wrote:

> pcc added a comment.
>
> In https://reviews.llvm.org/D38570#889021, @zturner wrote:
>
> > So if I understand the problem correctly, you don't have any issue with
> `ReplaceFile`'s behavior on the destination, only the source.  So, for
> example, you call `ReplaceFile`, it returns, and then some other process
> tries to open the //source// file and fails.
>
>
> Not quite. From the ThinLTO perspective I don't care about the behaviour
> on the source file before it is renamed, because all source files are
> temporary files whose names are unknown to other processes. The problem is
> with the behaviour on the source file after it has been renamed to the
> destination file. ThinLTO does this for each new cache file that it creates:
>
>   rename("thinlto-cache/temporary file name",
> "thinlto-cache/llvmcache-ABC123") // ABC123 is a unique hash that is
> computed from the linker's input files and command line options. There is a
> many-to-one mapping from hashes to object files (modulo collisions, but
> that's unlikely enough not to matter).
>
> In parallel, another process will be doing this in order to try and open
> an existing cache file:
>
>   open("thinlto-cache/llvmcache-ABC123") // This is expected to either
> succeed (cache hit) or return "no such file or directory" (cache miss).
>
> Where this breaks is when `rename` is implemented in terms of
> `ReplaceFile`. In pseudocode, here's what I think is going on in the
> important part of `ReplaceFile`:
>
>   void ReplaceFile(char *Src, char *Dst) {
>     HANDLE h = CreateFile(Src, no sharing); // (1) Src cannot be opened
>     SetFileInformationByHandle(h, Rename, Dst); // (2) Now Dst cannot be
> opened
>     CloseFile(h); // (3) Now Dst can be opened
>   }
>
> So if the `open` happens between 2 and 3, it will get a permission denied
> error, which is not what the caller of `open` is expecting.
>
>
> https://reviews.llvm.org/D38570
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171005/97e2556b/attachment.html>