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

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Wed May 4 06:18:10 PDT 2016


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. 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


More information about the llvm-commits mailing list