[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 10:32 AM Peter Collingbourne <peter at pcc.me.uk> wrote:

> On Wed, Oct 4, 2017 at 7:35 PM, Zachary Turner <zturner at google.com> wrote:
>
>> Ahh, i see what you’re saying. Dest *becomes* Source for a short time.
>>
>> The main thing I’m concerned about with this new implementation is that
>> it increases the complexity quite a bit, and in doing so makes the
>> semantics harder to reason about. How many additional weird codepaths in
>> the kernel is this tickling now? So there could be more (or at least
>> different) race conditions.
>>
>
> That was a concern for me as well, but it is somewhat mitigated by the
> fact that it is essentially a reimplementation of ReplaceFile, which is
> already part of the Windows API, so I'd expect the kernel code paths to be
> relatively well exercised. From MSDN: "An application can call ReplaceFile
> instead of calling separate functions to save the data to a new file,
> rename the original file using a temporary name, rename the new file to
> have the same name as the original file, and delete the original file."
> i.e. what my code is doing.
>
> Would it be possible for the ThinLTO processes to map a shared memory
>> region and synchronize on it instead of relying on flaky file system
>> semantics?
>>
>
> Not easily. The cache needs to be persisted between linker invocations, so
> I think there would need to be something on the file system. I suppose that
> you could imagine mmap'ing a large sparse file into every process and
> synchronizing on that, but there is also a mechanism for pruning old cache
> entries in order to save disk space (which may run concurrently with
> another process that is adding entries to the cache), so even if we did
> have an mmap'ed file, we'd need to arrange for the pruner to release pages
> to free storage, and to do so without races.
>

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171005/cda9d02c/attachment.html>