[PATCH] D48051: LTO: Work around a Windows kernel bug by keeping file handles open for memory mapped files.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

In https://reviews.llvm.org/D48051#1129025, @zturner wrote:

> In https://reviews.llvm.org/D48051#1129014, @pcc wrote:
>
> > In https://reviews.llvm.org/D48051#1128998, @zturner wrote:
> >
> > > Is there any we can just transfer ownership of the original handle (with `FILE_FLAG_DELETE_ON_CLOSE`) from whoever had it before to the `TempFile`.  This way we can just not close that handle until we're sure we're done with it.
> >
> >
> > I'm not sure what you mean. The `FILE_FLAG_DELETE_ON_CLOSE` handle is created and owned by `TempFile`. Do you mean transferring ownership from `TempFile` to `MemoryBuffer` when we map the file?
>
>
> Yea, sorry.  I'm just worried that by copying and duplicating the handle, and cancelling the delete on close, and all this other stuff we're just exposing ourselves to even more strange problems.  It seems like the "correct" thing to do is to just keep the original handle open until we're absolutely done with that file.


As I wrote in the commit message, that would prevent other processes from opening the file. What that would mean is that if we have two concurrent link processes they would not be able to share results.

I guess what we could do instead is exclusively use SetFileInformationByHandle/FileDispositionInfo for delete-on-close to avoid needing to do weird things to cancel `FILE_FLAG_DELETE_ON_CLOSE`. That should solve the sharing problem as well.

Also it would mean that we would need to change all other clients of `MemoryBuffer::getOpenFile{,Slice}()` to pass FD ownership to the `MemoryBuffer`. While that would seem to work for most in-tree clients there does appear to be at least one case where the FD is owned by external code: http://llvm-cs.pcc.me.uk/tools/gold/gold-plugin.cpp#498

So we'd probably either want the FD to be optionally owned or require the client to duplicate the handle if it doesn't own it. But that would mean that someone would still end up needing to duplicate on Windows in the non-owned case.


https://reviews.llvm.org/D48051