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

[Zachary Turner via llvm-commits]

We can’t transfer ownership of the handle in that case? With the flag
cleared we shouldn’t have to worry about the pending delete error
On Mon, Jun 11, 2018 at 5:07 PM Peter Collingbourne via Phabricator <
reviews at reviews.llvm.org> wrote:

> pcc added a comment.
>
> In https://reviews.llvm.org/D48051#1129142, @zturner wrote:
>
> > In https://reviews.llvm.org/D48051#1129127, @pcc wrote:
> >
> > > In https://reviews.llvm.org/D48051#1129126, @zturner wrote:
> > >
> > > > In https://reviews.llvm.org/D48051#1129121, @pcc wrote:
> > > >
> > > > > In https://reviews.llvm.org/D48051#1129117, @zturner wrote:
> > > > >
> > > > > > In https://reviews.llvm.org/D48051#1129112, @pcc wrote:
> > > > > >
> > > > > > > In https://reviews.llvm.org/D48051#1129090, @zturner wrote:
> > > > > > >
> > > > > > > > Do we have any kind of guarantee that one entity will
> outlive all other entities that may attempt to share the file?
> > > > > > >
> > > > > > >
> > > > > > > I don't think so, basically the process that creates a cache
> entry could exit at any time before or after any processes that might read
> the cache entry.
> > > > > >
> > > > > >
> > > > > > What happens if process A creates a cache entry, then exits,
> then process B tries to re-use the same cache entry?  Since it got closed
> (and deleted) before anyone could try to re-use it, does it just create it
> again?
> > > > >
> > > > >
> > > > > It doesn't normally get deleted because we clear the
> delete-on-close bit. It might get deleted later as a result of cache
> pruning, but that's a separate mechanism. But if it does get deleted
> through cache pruning, we do create it again.
> > > > >
> > > > > > It seems like it would be more efficient from a hit/miss ratio
> perspective if once a cache file was created, it would never be created
> again.
> > > > >
> > > > > That's exactly what happens, provided that the cache entry isn't
> pruned.
> > > >
> > > >
> > > > So after the entire link operation is finished, there are still
> cache files lying around on the hard drive (because their delete on close
> bit was cleared)?  If that's the case, can we just not even specify it in
> the first place?  What would be the implications of that?
> > >
> > >
> > > It would mean that if a link process is terminated forcefully or if
> there is a power cut, the link process's temporary files would be left on
> the disk.
> > >
> > > That's why I was thinking about using
> SetFileInformationByHandle/FileDispositionInfo instead of
> `FILE_FLAG_DELETE_ON_CLOSE` to manage the delete-on-close bit for temporary
> files. It would mean that if the process is terminated in the window
> between CreateFile and SetFileInformationByHandle the file would be left on
> the disk, but other than that we should basically get the same behaviour.
> Since getting terminated in that window is pretty unlikely, it seems okay
> to me.
> >
> >
> > Would the temporary files potentially be re-used on a subsequent link
> (if it were restarted by the user) and then cause mysterious failures?  If
> not, I'm honestly fine to just never set the flag at all.  We've been
> burned by it hard enough that who knows if we're triggering some additional
> strange / subtle codepath.  That said, your idea is probably fine.  At
> least it solves all the problems and eliminates all the duplicate handles
> and complexity.
>
>
> As I mentioned I think we'd still need to duplicate in cases where we
> didn't create the file handle (and there do seem to be at least a couple),
> so from an API simplicity perspective we might as well do it
> unconditionally. But I think we should at least be able to eliminate the
> magic for clearing the delete-on-close bit and the two-stage keep/close for
> temporary files.
>
>
> https://reviews.llvm.org/D48051
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180611/0ebacbe1/attachment.html>