[PATCH] D39710: Simplify unlinkAsync

[Rui Ueyama via llvm-commits]

On Tue, Nov 7, 2017 at 11:08 AM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:
>
> > ruiu added inline comments.
> >
> >
> > ================
> > Comment at: ELF/Filesystem.cpp:19
> >  #include "llvm/Support/FileSystem.h"
> > +#include <unistd.h>
> >
> > ----------------
> > Does all system have unistd.h?
>
> Looks like I have to check defined(HAVE_UNISTD_H). I will add that.
>
> >
> >
> > ================
> > Comment at: ELF/Filesystem.cpp:54
> >
> > -  // Remove TempPath in background.
> > -  runBackground([=] { ::remove(TempPath.str().str().c_str()); });
> > +  sys::fs::remove(Path);
> > +
> > ----------------
> > Is this safe on Windows? On Windows, you can't remove opened files.
>
> It depends on how they are opened. We use FILE_SHARE_DELETE in
> openFileForRead, so it should work.
>
> But in fact, remove being slow is a unix only problem, so I guess the
> best thing to do is to return early on windows. I will add that too.
>

Makes sense.

I believe that it is only a Windows thing that the entire process dies with
some error condition when the main thread exits while other threads are
running. So, we might be able to remove runInBackground and just detach a
thread that we create for calling close(2).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171107/91b430ad/attachment.html>