[Support][Modules] Removed prepareForGetLock and its usages. Ensured parent directory exists when creating lock file. (#189888)

Following #187372

clang/include/clang/Serialization/ModuleCache.h

@@ -29,10 +29,6 @@ class InMemoryModuleCache;
 /// operations the compiler might want to perform on the cache.
 class ModuleCache {
 public:
-  /// May perform any work that only needs to be performed once for multiple
-  /// calls \c getLock() with the same module filename.
-  virtual void prepareForGetLock(StringRef ModuleFilename) = 0;
-
   /// Returns lock for the given module file. The lock is initially unlocked.
   virtual std::unique_ptr<llvm::AdvisoryLock>
   getLock(StringRef ModuleFilename) = 0;

clang/lib/DependencyScanning/DependencyScannerImpl.cpp

@@ -657,7 +657,6 @@ struct AsyncModuleCompile : PPCallbacks {
       return;
     }
 
-    ModCache.prepareForGetLock(ModuleFileName);
     auto Lock = ModCache.getLock(ModuleFileName);
     bool Owned;
     llvm::Error LockErr = Lock->tryLock().moveInto(Owned);

clang/lib/DependencyScanning/InProcessModuleCache.cpp

@@ -84,8 +84,6 @@ class InProcessModuleCache : public ModuleCache {
 public:
   InProcessModuleCache(ModuleCacheEntries &Entries) : Entries(Entries) {}
 
-  void prepareForGetLock(StringRef Filename) override {}
-
   std::unique_ptr<llvm::AdvisoryLock> getLock(StringRef Filename) override {
     auto &Entry = [&]() -> ModuleCacheEntry & {
       std::lock_guard<std::mutex> Lock(Entries.Mutex);

clang/lib/Frontend/CompilerInstance.cpp

@@ -1529,7 +1529,6 @@ compileModuleBehindLockOrRead(CompilerInstance &ImportingInstance,
       << ModuleFileName << Module->Name;
 
   auto &ModuleCache = ImportingInstance.getModuleCache();
-  ModuleCache.prepareForGetLock(ModuleFileName);
 
   while (true) {
     auto Lock = ModuleCache.getLock(ModuleFileName);

clang/lib/Serialization/ModuleCache.cpp

@@ -169,16 +169,6 @@ class CrossProcessModuleCache : public ModuleCache {
   InMemoryModuleCache InMemory;
 
 public:
-  void prepareForGetLock(StringRef ModuleFilename) override {
-    // This is a compiler-internal input/output, let's bypass the sandbox.
-    auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
-
-    // FIXME: Do this in LockFileManager and only if the directory doesn't
-    // exist.
-    StringRef Dir = llvm::sys::path::parent_path(ModuleFilename);
-    llvm::sys::fs::create_directories(Dir);
-  }
-
   std::unique_ptr<llvm::AdvisoryLock>
   getLock(StringRef ModuleFilename) override {
     return std::make_unique<llvm::LockFileManager>(ModuleFilename);

llvm/lib/Support/LockFileManager.cpp

@@ -16,6 +16,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/IOSandbox.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
@@ -182,13 +183,32 @@ Expected<bool> LockFileManager::tryLock() {
   }
 
   // Create a lock file that is unique to this instance.
-  UniqueLockFileName = LockFileName;
-  UniqueLockFileName += "-%%%%%%%%";
   int UniqueLockFileID;
-  if (std::error_code EC = sys::fs::createUniqueFile(
-          UniqueLockFileName, UniqueLockFileID, UniqueLockFileName))
-    return createStringError(EC, "failed to create unique file " +
-                                     UniqueLockFileName);
+  {
+    SmallString<128> UniqueLockFilePattern = LockFileName;
+    UniqueLockFilePattern += "-%%%%%%%%";
+    SmallString<128> UniquePath;
+    std::error_code EC = sys::fs::createUniqueFile(
+        UniqueLockFilePattern, UniqueLockFileID, UniquePath);
+    if (EC == errc::no_such_file_or_directory) {
+      SmallString<128> Dir = sys::path::parent_path(UniqueLockFilePattern);
+      if (!Dir.empty()) {
+        if (std::error_code DirEC = sys::fs::create_directories(Dir))
+          return createStringError(DirEC,
+                                   "failed to create lock directory " + Dir);
+      }
+
+      // Retry creating lock file
+      EC = sys::fs::createUniqueFile(UniqueLockFilePattern, UniqueLockFileID,
+                                     UniquePath);
+    }
+
+    if (EC)
+      return createStringError(EC,
+                               "failed to create unique file " + UniquePath);
+
+    UniqueLockFileName = UniquePath;
+  }
 
   // Clean up the unique file on signal or scope exit.
   RemoveUniqueLockFileOnSignal RemoveUniqueFile(UniqueLockFileName);

llvm/unittests/Support/LockFileManagerTest.cpp

@@ -104,4 +104,32 @@ TEST(LockFileManagerTest, RelativePath) {
   ASSERT_FALSE(chdir(OrigPath));
 }
 
+TEST(LockFileManagerTest, NonExistentDirectory) {
+  TempDir LockFileManagerTestDir("LockFileManagerTestDir", /*Unique*/ true);
+
+  SmallString<64> NonExistentDir(LockFileManagerTestDir.path());
+  sys::path::append(NonExistentDir, "does_not_exist");
+
+  SmallString<64> LockedFile(NonExistentDir);
+  sys::path::append(LockedFile, "file");
+
+  SmallString<64> FileLock(LockedFile);
+  FileLock += ".lock";
+
+  // Ensure directory truly does not exist before test
+  EXPECT_FALSE(sys::fs::exists(NonExistentDir));
+
+  {
+    LockFileManager Locked(LockedFile);
+    EXPECT_THAT_EXPECTED(Locked.tryLock(), HasValue(true));
+
+    // Directory and lock file should now exist
+    EXPECT_TRUE(sys::fs::exists(NonExistentDir));
+    EXPECT_TRUE(sys::fs::exists(FileLock));
+  }
+
+  // After destruction, lock file should be removed
+  EXPECT_FALSE(sys::fs::exists(FileLock));
+}
+
 } // end anonymous namespace