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

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

In https://reviews.llvm.org/D48051#1129100, @pcc wrote:

> In https://reviews.llvm.org/D48051#1129078, @zturner wrote:
>
> > In https://reviews.llvm.org/D48051#1129059, @pcc wrote:
> >
> > > 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.
> >
> >
> > I don't think that would prevent other processes from opening the file.  Both processes just have to make sure they both specify `FILE_SHARE_DELETE`.
>
>
> As I wrote in my message to Bob that doesn't seem to work. According to process monitor the second process fails to open the file with "DELETE PENDING".


That's because a handle has been opened with `FILE_FLAG_DELETE_ON_CLOSE` and then subsequently closed.  As soon as you close a file that has been openend with that flag, no other opens can happen on the file.  That's why I suggested transferring ownership of it.

Are we guaranteed that there is one entity that will outlive all other entities who may wish to share the file?  As long as that one keeps the file open for its entire life, there shouldn't be any problems.


https://reviews.llvm.org/D48051