[PATCH] D19838: [LLVM-AR] Fixed bug where temporary file would be left behind every time an archive was updated

[Sean Silva via llvm-commits]

On Wed, May 4, 2016 at 6:18 AM, Rafael Espíndola <rafael.espindola at gmail.com
> wrote:

> On 3 May 2016 at 18:38, Cameron <cameron at moodycamel.com> wrote:
> > cameron314 added a comment.
> >
> > @silvas: The problem, unfortunately, is not with the way rename is
> implemented -- when the destination file has an open file handle on it,
> there's only so much that can be done regardless of the method.
>
> I wonder if we could not implement it as an atomic rename or fail.


AFAIK (and I'm certainly not an expert, but Michael and I did look at this
in detail), the windows filesystem fundamentally does not have this
semantic. In Unix, rename is a modification of a namespace of the vfs. On
windows, files work differently and there isn't really an analog to this
operation that can be atomic. (they may actually be a really low-level way
of doing this that has the right semantics; but at least the Win32 API
doesn't have it).

-- Sean Silva


> The
> issue right now is that if we forget an open file it will leave a
> rename copy around. That is a bug in the caller, but we would noticed
> it way faster if the rename actually failed.
>
> That is, should we we stop using ReplaceFileW and use only MoveFileEx
> in rename? (that should be another patch). That would be a semantic
> revert of r250046.
>
> If we can detect that a MoveFileEx failed because the file is open we
> should just loop forever. It is either some scanner that has it open
> and it will go away or it is a bug in llvm and we will notice it.
>
> ccing the folks in https://llvm.org/bugs/show_bug.cgi?id=27386.
>
>
> > @rafael: I could do that. It made more sense to me that it be linked to
> the archive (since once freed, the archive is no longer valid).
>
> A bit of history: the classes in lib/Object used to own the buffers,
> since as you noticed that is simpler for some applications. The
> problem is that we always found cases where someone had a buffer,
> wanted to use the classes to extract some info, but still keep the
> buffer, so we switched them to be non-owning.
>
> Having optional ownership in llvm is something we avoid in general.
>
> So I think passing the buffer of the original archive in is the best
> solution.
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160504/84861c0c/attachment.html>