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

[Zachary Turner via llvm-commits]

On Thu, Oct 5, 2017 at 11:27 AM Reid Kleckner <rnk at google.com> wrote:

> On Thu, Oct 5, 2017 at 10:51 AM, Zachary Turner <zturner at google.com>
> wrote:
>
>> What I mean is that you could have a system-wide named mutex with a
>> unique name that involves a 128-bit GUID or something.  All ThinLTO linker
>> processes could agree on this name from now until forever.  This would
>> presist across linker invocations because the name of the mutex never
>> changes.  This would synchronize access to a single "cache control file"
>> that also had a globally unique (and unchanging) name.  So the algorithm
>> for opening a file from the cache is:
>>
>> 1) Acquire the mutex
>> 2) Try to open the file and either succeed or fail
>> 3) Close the mutex.
>>
>> and the algorithm for adding a new file to the cache is:
>>
>> 1) Acquire the mutex
>> 2) Do whatever it takes to get a new file into the cache
>> 3) Close the mutex
>>
>> This should never race.
>>
>
> This patch makes our rename more POSIX-y. That seems like an objectively
> good thing, regardless of what the best way to implement a shared,
> persistent cache is on Windows. In the long run, this sounds like something
> that could replace LLVM's probably hopelessly broken lock file
> implementation on Windows.
>

I guess this is a tangent, but if we want to implement robust lock files on
Windows, I'd suggest just *actually* locking the file
<https://msdn.microsoft.com/en-us/library/windows/desktop/aa365202%28v=vs.85%29.aspx?f=255&MSPPError=-2147217396>
and
providing an abstraction that hides the details of how a file system lock
works, and on Windows just uses this function and on posix relies on a
separate file.

Can we at least *try* to just call MoveFileEx first?  If it fails then we
can fall back to the more complicated code-path.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171005/39a10224/attachment.html>