<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 4, 2016 at 6:18 AM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 3 May 2016 at 18:38, Cameron <<a href="mailto:cameron@moodycamel.com">cameron@moodycamel.com</a>> wrote:<br>
> cameron314 added a comment.<br>
><br>
> @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.<br>
<br>
</span>I wonder if we could not implement it as an atomic rename or fail.</blockquote><div><br></div><div>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).</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> The<br>
issue right now is that if we forget an open file it will leave a<br>
rename copy around. That is a bug in the caller, but we would noticed<br>
it way faster if the rename actually failed.<br>
<br>
That is, should we we stop using ReplaceFileW and use only MoveFileEx<br>
in rename? (that should be another patch). That would be a semantic<br>
revert of r250046.<br>
<br>
If we can detect that a MoveFileEx failed because the file is open we<br>
should just loop forever. It is either some scanner that has it open<br>
and it will go away or it is a bug in llvm and we will notice it.<br>
<br>
ccing the folks in <a href="https://llvm.org/bugs/show_bug.cgi?id=27386" rel="noreferrer" target="_blank">https://llvm.org/bugs/show_bug.cgi?id=27386</a>.<br>
<span class=""><br>
<br>
> @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).<br>
<br>
</span>A bit of history: the classes in lib/Object used to own the buffers,<br>
since as you noticed that is simpler for some applications. The<br>
problem is that we always found cases where someone had a buffer,<br>
wanted to use the classes to extract some info, but still keep the<br>
buffer, so we switched them to be non-owning.<br>
<br>
Having optional ownership in llvm is something we avoid in general.<br>
<br>
So I think passing the buffer of the original archive in is the best solution.<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div></div>