[Dependency Scanning] Isolate shared dependency scanner state

Using mutual exclusion, ensuring that multiple threads executing dependency scans do not encounter data races on shared mutable state.

There are two layers with shared state where we need to be careful:
- `DependencyScanningTool`, as the main entity that scanning clients interact with. This tool instantiates compiler instances for individual scans, computing a scanning invocation hash. It needs to remember those instances for future use, and when creating instances it needs to reset LLVM argument processor's global state, meaning all uses of argument processing must be in a critical section.

- `SwiftDependencyScanningService`, as the main cache where dependency scanning results are stored. Each individual scan instantiates a `ModuleDependenciesCache`, which uses the scanning service as the underlying storage. The services' storage is segmented to storing dependencies discovered in a scan with a given context hash, which means two different scanning invocations running at the same time will be accessing different locations in its storage, thus not requiring synchronization. But the service still has some shared state that must be protected, such as the collection of discovered source modules, and the map used to query context-hash-specific underlying cache storage.

Author: artemcm

include/swift/AST/ModuleDependencies.h

@@ -25,6 +25,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Mutex.h"
 #include <string>
 #include <vector>
 #include <unordered_map>
@@ -580,18 +581,20 @@ class SwiftDependencyScanningService {
   llvm::StringMap<std::unique_ptr<ContextSpecificGlobalCacheState>>
       ContextSpecificCacheMap;
 
-  /// The current context hash configuration
-  Optional<std::string> CurrentContextHash;
-
   /// The context hashes used by scanners using this cache, in the order in
   /// which they were used
   std::vector<std::string> AllContextHashes;
 
+  /// Shared state mutual-exclusivity lock
+  llvm::sys::SmartMutex<true> ScanningServiceGlobalLock;
+
   /// Retrieve the dependencies map that corresponds to the given dependency
   /// kind.
-  ModuleNameToDependencyMap &getDependenciesMap(ModuleDependencyKind kind);
+  ModuleNameToDependencyMap &getDependenciesMap(ModuleDependencyKind kind,
+                                                StringRef scanContextHash);
   const ModuleNameToDependencyMap &
-  getDependenciesMap(ModuleDependencyKind kind) const;
+  getDependenciesMap(ModuleDependencyKind kind,
+                     StringRef scanContextHash) const;
 
 public:
   SwiftDependencyScanningService();
@@ -613,8 +616,8 @@ class SwiftDependencyScanningService {
     return *SharedFilesystemCache;
   }
 
-  llvm::StringSet<>& getAlreadySeenClangModules() {
-    return getCurrentCache()->alreadySeenClangModules;
+  llvm::StringSet<>& getAlreadySeenClangModules(StringRef scanningContextHash) {
+    return getCacheForScanningContextHash(scanningContextHash)->alreadySeenClangModules;
   }
 
   /// Wrap the filesystem on the specified `CompilerInstance` with a
@@ -630,7 +633,7 @@ class SwiftDependencyScanningService {
 
   /// Configure the current state of the cache to respond to queries
   /// for the specified scanning context hash.
-  void configureForContextHash(std::string scanningContextHash);
+  void configureForContextHash(StringRef scanningContextHash);
 
   /// Return context hashes of all scanner invocations that have used
   /// this cache instance.
@@ -640,15 +643,12 @@ class SwiftDependencyScanningService {
 
   /// Whether we have cached dependency information for the given module.
   bool hasDependency(StringRef moduleName,
-                     Optional<ModuleDependencyKind> kind) const;
-
-  /// Return a pointer to the context-specific cache state of the current
-  /// scanning action.
-  ContextSpecificGlobalCacheState *getCurrentCache() const;
+                     Optional<ModuleDependencyKind> kind,
+                     StringRef scanContextHash) const;
 
   /// Return a pointer to the cache state of the specified context hash.
   ContextSpecificGlobalCacheState *
-  getCacheForScanningContextHash(StringRef scanningContextHash) const;
+  getCacheForScanningContextHash(StringRef scanContextHash) const;
 
   /// Look for source-based module dependency details
   Optional<const ModuleDependencyInfo*>
@@ -659,15 +659,22 @@ class SwiftDependencyScanningService {
   /// \returns the cached result, or \c None if there is no cached entry.
   Optional<const ModuleDependencyInfo*>
   findDependency(StringRef moduleName,
-                 Optional<ModuleDependencyKind> kind) const;
+                 Optional<ModuleDependencyKind> kind,
+                 StringRef scanContextHash) const;
 
   /// Record dependencies for the given module.
   const ModuleDependencyInfo *recordDependency(StringRef moduleName,
-                                               ModuleDependencyInfo dependencies);
+                                               ModuleDependencyInfo dependencies,
+                                               StringRef scanContextHash);
+
+  /// Record source-module dependencies for the given module.
+  const ModuleDependencyInfo *recordSourceDependency(StringRef moduleName,
+                                                     ModuleDependencyInfo dependencies);
 
   /// Update stored dependencies for the given module.
   const ModuleDependencyInfo *updateDependency(ModuleDependencyID moduleID,
-                                               ModuleDependencyInfo dependencies);
+                                               ModuleDependencyInfo dependencies,
+                                               StringRef scanContextHash);
 
   /// Reference the list of all module dependencies that are not source-based
   /// modules (i.e. interface dependencies, binary dependencies, clang
@@ -727,7 +734,7 @@ class ModuleDependenciesCache {
     return clangScanningTool;
   }
   llvm::StringSet<>& getAlreadySeenClangModules() {
-    return globalScanningService.getAlreadySeenClangModules();
+    return globalScanningService.getAlreadySeenClangModules(scannerContextHash);
   }
 
   /// Look for module dependencies for a module with the given name

include/swift/DependencyScan/DependencyScanningTool.h

@@ -115,6 +115,9 @@ class DependencyScanningTool {
   /// command-line options specified in the batch scan input entry.
   std::unique_ptr<CompilerArgInstanceCacheMap> VersionedPCMInstanceCacheCache;
 
+  /// Shared state mutual-exclusivity lock
+  llvm::sys::SmartMutex<true> DependencyScanningToolStateLock;
+
   /// A shared consumer that accumulates encountered diagnostics.
   DependencyScannerDiagnosticCollectingConsumer CDC;
   llvm::BumpPtrAllocator Alloc;

lib/AST/ModuleDependencies.cpp

@@ -251,13 +251,6 @@ void SwiftDependencyScanningService::overlaySharedFilesystemCacheForCompilation(
  Instance.getSourceMgr().setFileSystem(depFS);
 }
 
-SwiftDependencyScanningService::ContextSpecificGlobalCacheState *
-SwiftDependencyScanningService::getCurrentCache() const {
-  assert(CurrentContextHash.has_value() &&
-         "Global Module Dependencies Cache not configured with Triple.");
-  return getCacheForScanningContextHash(CurrentContextHash.value());
-}
-
 SwiftDependencyScanningService::ContextSpecificGlobalCacheState *
 SwiftDependencyScanningService::getCacheForScanningContextHash(StringRef scanningContextHash) const {
   auto contextSpecificCache = ContextSpecificCacheMap.find(scanningContextHash);
@@ -269,8 +262,8 @@ SwiftDependencyScanningService::getCacheForScanningContextHash(StringRef scannin
 
 const ModuleNameToDependencyMap &
 SwiftDependencyScanningService::getDependenciesMap(
-    ModuleDependencyKind kind) const {
-  auto contextSpecificCache = getCurrentCache();
+    ModuleDependencyKind kind, StringRef scanContextHash) const {
+  auto contextSpecificCache = getCacheForScanningContextHash(scanContextHash);
   auto it = contextSpecificCache->ModuleDependenciesMap.find(kind);
   assert(it != contextSpecificCache->ModuleDependenciesMap.end() &&
          "invalid dependency kind");
@@ -279,40 +272,38 @@ SwiftDependencyScanningService::getDependenciesMap(
 
 ModuleNameToDependencyMap &
 SwiftDependencyScanningService::getDependenciesMap(
-    ModuleDependencyKind kind) {
-  auto contextSpecificCache = getCurrentCache();
+    ModuleDependencyKind kind, StringRef scanContextHash) {
+  llvm::sys::SmartScopedLock<true> Lock(ScanningServiceGlobalLock);
+  auto contextSpecificCache = getCacheForScanningContextHash(scanContextHash);
   auto it = contextSpecificCache->ModuleDependenciesMap.find(kind);
   assert(it != contextSpecificCache->ModuleDependenciesMap.end() &&
          "invalid dependency kind");
   return it->second;
 }
 
-void SwiftDependencyScanningService::configureForContextHash(std::string scanningContextHash) {
+void SwiftDependencyScanningService::configureForContextHash(StringRef scanningContextHash) {
   auto knownContext = ContextSpecificCacheMap.find(scanningContextHash);
-  if (knownContext != ContextSpecificCacheMap.end()) {
-    // Set the current context and leave the rest as-is
-    CurrentContextHash = scanningContextHash;
-  } else {
-    // First time scanning with this triple, initialize target-specific state.
+  if (knownContext == ContextSpecificCacheMap.end()) {
+    // First time scanning with this context, initialize context-specific state.
     std::unique_ptr<ContextSpecificGlobalCacheState> contextSpecificCache =
         std::make_unique<ContextSpecificGlobalCacheState>();
     for (auto kind = ModuleDependencyKind::FirstKind;
          kind != ModuleDependencyKind::LastKind; ++kind) {
       contextSpecificCache->ModuleDependenciesMap.insert({kind, ModuleNameToDependencyMap()});
     }
-
-    ContextSpecificCacheMap.insert({scanningContextHash, std::move(contextSpecificCache)});
-    CurrentContextHash = scanningContextHash;
-    AllContextHashes.push_back(scanningContextHash);
+    llvm::sys::SmartScopedLock<true> Lock(ScanningServiceGlobalLock);
+    ContextSpecificCacheMap.insert({scanningContextHash.str(), std::move(contextSpecificCache)});
+    AllContextHashes.push_back(scanningContextHash.str());
   }
 }
 
 Optional<const ModuleDependencyInfo*> SwiftDependencyScanningService::findDependency(
-    StringRef moduleName, Optional<ModuleDependencyKind> kind) const {
+    StringRef moduleName, Optional<ModuleDependencyKind> kind,
+    StringRef scanningContextHash) const {
   if (!kind) {
     for (auto kind = ModuleDependencyKind::FirstKind;
          kind != ModuleDependencyKind::LastKind; ++kind) {
-      auto dep = findDependency(moduleName, kind);
+      auto dep = findDependency(moduleName, kind, scanningContextHash);
       if (dep.has_value())
         return dep.value();
     }
@@ -324,7 +315,7 @@ Optional<const ModuleDependencyInfo*> SwiftDependencyScanningService::findDepend
     return findSourceModuleDependency(moduleName);
   }
 
-  const auto &map = getDependenciesMap(kind.value());
+  const auto &map = getDependenciesMap(kind.value(), scanningContextHash);
   auto known = map.find(moduleName);
   if (known != map.end())
     return &(known->second);
@@ -343,44 +334,55 @@ SwiftDependencyScanningService::findSourceModuleDependency(
 }
 
 bool SwiftDependencyScanningService::hasDependency(
-    StringRef moduleName, Optional<ModuleDependencyKind> kind) const {
-  return findDependency(moduleName, kind).has_value();
+    StringRef moduleName, Optional<ModuleDependencyKind> kind,
+    StringRef scanContextHash) const {
+  return findDependency(moduleName, kind, scanContextHash).has_value();
 }
 
 const ModuleDependencyInfo *SwiftDependencyScanningService::recordDependency(
-    StringRef moduleName, ModuleDependencyInfo dependencies) {
+    StringRef moduleName, ModuleDependencyInfo dependencies,
+    StringRef scanContextHash) {
   auto kind = dependencies.getKind();
   // Source-based dependencies are recorded independently of the invocation's
   // target triple.
-  if (kind == swift::ModuleDependencyKind::SwiftSource) {
-    assert(SwiftSourceModuleDependenciesMap.count(moduleName) == 0 &&
-           "Attempting to record duplicate SwiftSource dependency.");
-    SwiftSourceModuleDependenciesMap.insert(
-        {moduleName, std::move(dependencies)});
-    AllSourceModules.push_back({moduleName.str(), kind});
-    return &(SwiftSourceModuleDependenciesMap.find(moduleName)->second);
-  }
+  if (kind == swift::ModuleDependencyKind::SwiftSource)
+    return recordSourceDependency(moduleName, std::move(dependencies));
 
   // All other dependencies are recorded according to the target triple of the
   // scanning invocation that discovers them.
-  auto &map = getDependenciesMap(kind);
+  auto &map = getDependenciesMap(kind, scanContextHash);
   map.insert({moduleName, dependencies});
   return &(map[moduleName]);
 }
 
+const ModuleDependencyInfo *SwiftDependencyScanningService::recordSourceDependency(
+    StringRef moduleName, ModuleDependencyInfo dependencies) {
+  llvm::sys::SmartScopedLock<true> Lock(ScanningServiceGlobalLock);
+  auto kind = dependencies.getKind();
+  assert(kind == swift::ModuleDependencyKind::SwiftSource && "Expected source module dependncy info");
+  assert(SwiftSourceModuleDependenciesMap.count(moduleName) == 0 &&
+         "Attempting to record duplicate SwiftSource dependency.");
+  SwiftSourceModuleDependenciesMap.insert(
+      {moduleName, std::move(dependencies)});
+  AllSourceModules.push_back({moduleName.str(), kind});
+  return &(SwiftSourceModuleDependenciesMap.find(moduleName)->second);
+}
+
 const ModuleDependencyInfo *SwiftDependencyScanningService::updateDependency(
-    ModuleDependencyID moduleID, ModuleDependencyInfo dependencies) {
+    ModuleDependencyID moduleID, ModuleDependencyInfo dependencies,
+    StringRef scanningContextHash) {
   auto kind = dependencies.getKind();
   // Source-based dependencies
   if (kind == swift::ModuleDependencyKind::SwiftSource) {
+    llvm::sys::SmartScopedLock<true> Lock(ScanningServiceGlobalLock);
     assert(SwiftSourceModuleDependenciesMap.count(moduleID.first) == 1 &&
            "Attempting to update non-existing Swift Source dependency.");
     auto known = SwiftSourceModuleDependenciesMap.find(moduleID.first);
     known->second = std::move(dependencies);
     return &(known->second);
   }
 
-  auto &map = getDependenciesMap(moduleID.second);
+  auto &map = getDependenciesMap(moduleID.second, scanningContextHash);
   auto known = map.find(moduleID.first);
   assert(known != map.end() && "Not yet added to map");
   known->second = std::move(dependencies);
@@ -422,9 +424,10 @@ ModuleDependenciesCache::ModuleDependenciesCache(
 Optional<const ModuleDependencyInfo*>
 ModuleDependenciesCache::findDependency(
     StringRef moduleName, Optional<ModuleDependencyKind> kind) const {
-  auto optionalDep = globalScanningService.findDependency(moduleName, kind);
-  // During a scan, only produce the cached source module info for the current module
-  // under scan.
+  auto optionalDep = globalScanningService.findDependency(moduleName, kind,
+                                                          scannerContextHash);
+  // During a scan, only produce the cached source module info for the current
+  // module under scan.
   if (optionalDep.hasValue()) {
     auto dep = optionalDep.getValue();
     if (dep->getAsSwiftSourceModule() &&
@@ -446,7 +449,8 @@ void ModuleDependenciesCache::recordDependency(
     StringRef moduleName, ModuleDependencyInfo dependencies) {
   auto dependenciesKind = dependencies.getKind();
   const ModuleDependencyInfo *recordedDependencies =
-        globalScanningService.recordDependency(moduleName, dependencies);
+        globalScanningService.recordDependency(moduleName, dependencies,
+                                               scannerContextHash);
 
   auto &map = getDependencyReferencesMap(dependenciesKind);
   assert(map.count(moduleName) == 0 && "Already added to map");
@@ -455,7 +459,9 @@ void ModuleDependenciesCache::recordDependency(
 
 void ModuleDependenciesCache::updateDependency(
     ModuleDependencyID moduleID, ModuleDependencyInfo dependencyInfo) {
-  const ModuleDependencyInfo *updatedDependencies = globalScanningService.updateDependency(moduleID, dependencyInfo);
+  const ModuleDependencyInfo *updatedDependencies =
+    globalScanningService.updateDependency(moduleID, dependencyInfo,
+                                           scannerContextHash);
   auto &map = getDependencyReferencesMap(moduleID.second);
   auto known = map.find(moduleID.first);
   if (known != map.end())

lib/DependencyScan/DependencyScanningTool.cpp

@@ -26,8 +26,13 @@
 namespace swift {
 namespace dependencies {
 
+// Global mutex for target info queries since they are executed separately .
+llvm::sys::SmartMutex<true> TargetInfoMutex;
+
 llvm::ErrorOr<swiftscan_string_ref_t> getTargetInfo(ArrayRef<const char *> Command,
                                                     const char *main_executable_path) {
+  llvm::sys::SmartScopedLock<true> Lock(TargetInfoMutex);
+
   // We must reset option occurrences because we are handling an unrelated
   // command-line to those possibly parsed before using the same tool.
   // We must do so because LLVM options parsing is done using a managed
@@ -178,6 +183,7 @@ DependencyScanningTool::getDependencies(
 }
 
 void DependencyScanningTool::serializeCache(llvm::StringRef path) {
+  llvm::sys::SmartScopedLock<true> Lock(DependencyScanningToolStateLock);
   SourceManager SM;
   DiagnosticEngine Diags(SM);
   Diags.addConsumer(CDC);
@@ -186,6 +192,7 @@ void DependencyScanningTool::serializeCache(llvm::StringRef path) {
 }
 
 bool DependencyScanningTool::loadCache(llvm::StringRef path) {
+  llvm::sys::SmartScopedLock<true> Lock(DependencyScanningToolStateLock);
   SourceManager SM;
   DiagnosticEngine Diags(SM);
   Diags.addConsumer(CDC);
@@ -210,6 +217,10 @@ void DependencyScanningTool::resetDiagnostics() {
 llvm::ErrorOr<std::unique_ptr<CompilerInstance>>
 DependencyScanningTool::initScannerForAction(
     ArrayRef<const char *> Command) {
+  // The remainder of this method operates on shared state in the
+  // scanning service and global LLVM state with:
+  // llvm::cl::ResetAllOptionOccurrences
+  llvm::sys::SmartScopedLock<true> Lock(DependencyScanningToolStateLock);
   auto instanceOrErr = initCompilerInstanceForScan(Command);
   if (instanceOrErr.getError())
     return instanceOrErr;