LTO: make tempfiles in LTO cache subject to pruning

[inglorion]

Summary

CachePruning only considers files with names starting in
"llvmcache-". The LTO cache creates temporary files starting in
"Thin-". This results in surprising behavior, such as running out of
disk space although the cache pruning policy is set to limit the cache
directory to a reasonable size. This change causes the LTO cache to
create temporary files with names that will be handled by the pruner,
avoiding the problem.

Fixes PR38618.

[pcc]

The temporary files are deliberately named differently from the cache entries in order to prevent cache pruners from racing with processes that create cache files. If we allow these deletions it would have the effect of causing the call to TempFile::keep to sometimes return no such file or directory, so I'd at least expect to see code handling that case.

Do you know why the temporary files are being left on disk? I wouldn't have expected that to happen because they are being created with delete-on-close.

[inglorion]

Do you know why the temporary files are being left on disk? I wouldn't have expected that to happen because they are being created with delete-on-close.

Yes. The problem is that there are two races with delete on close. More discussion and analysis is on PR38616, but it boils down to:

1. The first race is between when we open the file and we set the DeleteFile flag. We can't use the alternative of opening the file with the FILE_DELETE_ON_CLOSE flag, because we can't clear that one (and so there would be no way to keep a temporary file).

2. The second race is between when we clear the DeleteFile flag and we rename the file (in the renaming version of TempFile::keep()). We can't atomically perform the flag change and the rename, and we can't do the flag change after the name because this causes everyone who opens the file with its non-temporary name to fail with ERROR_ACCESS_DENIED until we clear the flag (this breaks large parts of the LLVM test suite).

Anyway, thanks for pointing out the reason why the temporary files are named so as to avoid the pruner. I'll do some more thinking.

[pcc]

Hmm, I was aware of those races, but it's a little surprising that they would be the cause of a significant number of files being left around (since the files exist with their temporary names for the duration that the LLVM backend is running). I could buy that the Windows filesystem is slow enough that the sum of the racy windows would make up a significant fraction of that time, though.

[inglorion]

Since D94962 was landed, we don't need this one anymore.

llvm/lib/LTO/Caching.cpp

struct CacheStream : NativeObjectStream {
 
         AddBuffer(Task, std::move(*MBOrErr));
       }
     };
 
     return [=](size_t Task) -> std::unique_ptr<NativeObjectStream> {
       // Write to a temporary to avoid race condition
       SmallString<64> TempFilenameModel;
-      sys::path::append(TempFilenameModel, CacheDirectoryPath, "Thin-%%%%%%.tmp.o");
+      sys::path::append(TempFilenameModel, CacheDirectoryPath,
+                        "llvmcache-%%%%%%.tmp.o");
       Expected<sys::fs::TempFile> Temp = sys::fs::TempFile::create(
           TempFilenameModel, sys::fs::owner_read | sys::fs::owner_write);
       if (!Temp) {
         errs() << "Error: " << toString(Temp.takeError()) << "\n";
         report_fatal_error("ThinLTO: Can't get a temporary file");
       }
 
       // This CacheStream will move the temporary file into the cache when done.
       return llvm::make_unique<CacheStream>(
           llvm::make_unique<raw_fd_ostream>(Temp->FD, /* ShouldClose */ false),
           AddBuffer, std::move(*Temp), EntryPath.str(), Task);
     };
   };
 }