[PATCH] D39710: Simplify unlinkAsync

[Rafael Avila de Espindola via llvm-commits]

Rui Ueyama <ruiu at google.com> writes:

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

Nice :-)

Cheers,
Rafael