llvm.org GIT mirror llvm / a3886c1
LTO: Try to open cache files before renaming them. It appears that a potential race between the cache client and the cache pruner that I thought was unlikely actually happened in practice [1]. Try to avoid the race condition by opening the temporary file before renaming it. Do this only on non-Windows platforms because we cannot rename open files on Windows using the sys::fs::rename function. [1] https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.memory%2FLinux_CFI%2F1610%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout Differential Revision: https://reviews.llvm.org/D37410 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@312567 91177308-0d34-0410-b5e6-96231b3b80d8 Peter Collingbourne 3 years ago
4 changed file(s) with 31 addition(s) and 13 deletion(s). Raw diff Collapse all Expand all
2323 /// This type defines the callback to add a pre-existing native object file
2424 /// (e.g. in a cache).
2525 ///
26 /// MB->getBufferIdentifier() is a valid path for the file at the time that it
27 /// was opened, but clients should prefer to access MB directly in order to
28 /// avoid a potential race condition.
26 /// Path is generally expected to be a valid path for the file at the point when
27 /// the AddBufferFn function is called, but clients should prefer to access MB
28 /// directly in order to avoid a potential race condition.
2929 ///
3030 /// Buffer callbacks must be thread safe.
31 typedef std::function MB)>
31 typedef std::function MB,
32 StringRef Path)>
3233 AddBufferFn;
3334
3435 /// Create a local file system cache which uses the given cache directory and
3535 ErrorOr> MBOrErr =
3636 MemoryBuffer::getFile(EntryPath);
3737 if (MBOrErr) {
38 AddBuffer(Task, std::move(*MBOrErr));
38 AddBuffer(Task, std::move(*MBOrErr), EntryPath);
3939 return AddStreamFn();
4040 }
4141
5959 EntryPath(std::move(EntryPath)), Task(Task) {}
6060
6161 ~CacheStream() {
62 // Make sure the file is closed before committing it.
63 OS.reset();
64
65 #ifdef _WIN32
66 // Rename the file first on Windows because we cannot rename an open
67 // file on that platform using the sys::fs::rename function.
6268 // FIXME: This code could race with the cache pruner, but it is unlikely
6369 // that the cache pruner will choose to remove a newly created file.
64
65 // Make sure the file is closed before committing it.
66 OS.reset();
67 // This is atomic on POSIX systems.
70 // We should look at using the SetFileInformationByHandle function to
71 // rename the file while it is open.
6872 if (auto EC = sys::fs::rename(TempFilename, EntryPath))
6973 report_fatal_error(Twine("Failed to rename temporary file ") +
7074 TempFilename + ": " + EC.message() + "\n");
7175
7276 ErrorOr> MBOrErr =
7377 MemoryBuffer::getFile(EntryPath);
78 #else
79 // Open the file first to avoid racing with a cache pruner.
80 ErrorOr> MBOrErr =
81 MemoryBuffer::getFile(TempFilename);
82
83 // This is atomic on POSIX systems.
84 if (auto EC = sys::fs::rename(TempFilename, EntryPath))
85 report_fatal_error(Twine("Failed to rename temporary file ") +
86 TempFilename + ": " + EC.message() + "\n");
87 #endif
88
7489 if (!MBOrErr)
7590 report_fatal_error(Twine("Failed to open cache file ") + EntryPath +
7691 ": " + MBOrErr.getError().message() + "\n");
77 AddBuffer(Task, std::move(*MBOrErr));
92 AddBuffer(Task, std::move(*MBOrErr), EntryPath);
7893 }
7994 };
8095
907907 llvm::make_unique(FD, true));
908908 };
909909
910 auto AddBuffer = [&](size_t Task, std::unique_ptr MB) {
910 auto AddBuffer = [&](size_t Task, std::unique_ptr MB,
911 StringRef Path) {
911912 // Note that this requires that the memory buffers provided to AddBuffer are
912913 // backed by a file.
913 Filenames[Task] = MB->getBufferIdentifier();
914 Filenames[Task] = Path;
914915 };
915916
916917 NativeObjectCache Cache;
295295 return llvm::make_unique(std::move(S));
296296 };
297297
298 auto AddBuffer = [&](size_t Task, std::unique_ptr MB) {
298 auto AddBuffer = [&](size_t Task, std::unique_ptr MB,
299 StringRef Path) {
299300 *AddStream(Task)->OS << MB->getBuffer();
300301 };
301302