From e9e247c09b76dc6813c70d2c7007438c830bea56 Mon Sep 17 00:00:00 2001
From: Thiabaud Engelbrecht <thiabaud@google.com>
Date: Thu, 05 Jun 2025 11:18:54 -0700
Subject: [PATCH] [ssm] Fix race condition

This CL fixes a threading issue, by using locks to prevent racy access.

Bug: 421471016
Change-Id: If41796b9eb49aa3f27df175c6be198f47947c4c0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6620528
Reviewed-by: Jean-Philippe Gravel <jpgravel@chromium.org>
Commit-Queue: Thiabaud Engelbrecht <thiabaud@google.com>
Cr-Commit-Position: refs/heads/main@{#1470083}
---

diff --git a/base/profiler/module_cache.cc b/base/profiler/module_cache.cc
index 46f0f19..31d4753 100644
--- a/base/profiler/module_cache.cc
+++ b/base/profiler/module_cache.cc
@@ -90,6 +90,7 @@
   for (const std::unique_ptr<const Module>& module : native_modules_) {
     result.push_back(module.get());
   }
+  base::AutoLock locker(lock_);
   for (const std::unique_ptr<const Module>& module : non_native_modules_) {
     result.push_back(module.get());
   }
@@ -103,6 +104,7 @@
   flat_set<const Module*> defunct_modules_set(defunct_modules.begin(),
                                               defunct_modules.end());
 
+  base::AutoLock locker(lock_);
   // Reorder the modules to be removed to the last slots in the set, then move
   // them to the inactive modules, then erase the moved-from modules from the
   // set. This is a variation on the standard erase-remove idiom, which is
@@ -153,9 +155,12 @@
 
 const ModuleCache::Module* ModuleCache::GetExistingModuleForAddress(
     uintptr_t address) const {
-  const auto non_native_module_loc = non_native_modules_.find(address);
-  if (non_native_module_loc != non_native_modules_.end()) {
-    return non_native_module_loc->get();
+  {
+    base::AutoLock locker(lock_);
+    const auto non_native_module_loc = non_native_modules_.find(address);
+    if (non_native_module_loc != non_native_modules_.end()) {
+      return non_native_module_loc->get();
+    }
   }
 
   const auto native_module_loc = native_modules_.find(address);
diff --git a/base/profiler/module_cache.h b/base/profiler/module_cache.h
index 0a3ca02..14de5781 100644
--- a/base/profiler/module_cache.h
+++ b/base/profiler/module_cache.h
@@ -15,6 +15,8 @@
 #include "base/containers/flat_set.h"
 #include "base/files/file_path.h"
 #include "base/memory/raw_ptr.h"
+#include "base/synchronization/lock.h"
+#include "base/thread_annotations.h"
 #include "build/build_config.h"
 
 #if BUILDFLAG(IS_WIN)
@@ -94,8 +96,8 @@
   // Gets the module containing |address| or nullptr if |address| is not within
   // a module. The returned module remains owned by and has the same lifetime as
   // the ModuleCache object.
-  const Module* GetModuleForAddress(uintptr_t address);
-  std::vector<const Module*> GetModules() const;
+  const Module* GetModuleForAddress(uintptr_t address) LOCKS_EXCLUDED(lock_);
+  std::vector<const Module*> GetModules() const LOCKS_EXCLUDED(lock_);
 
   // Updates the set of non-native modules maintained by the
   // ModuleCache. Non-native modules represent regions of non-native executable
@@ -116,7 +118,8 @@
   // the same call.
   void UpdateNonNativeModules(
       const std::vector<const Module*>& defunct_modules,
-      std::vector<std::unique_ptr<const Module>> new_modules);
+      std::vector<std::unique_ptr<const Module>> new_modules)
+      LOCKS_EXCLUDED(lock_);
 
   // Adds a custom native module to the cache. This is intended to support
   // native modules that require custom handling. In general, native modules
@@ -144,7 +147,8 @@
   // NOTE: Only users that create their own modules and need control over native
   // module creation should use this function. Everyone else should use
   // GetModuleForAddress().
-  const Module* GetExistingModuleForAddress(uintptr_t address) const;
+  const Module* GetExistingModuleForAddress(uintptr_t address) const
+      LOCKS_EXCLUDED(lock_);
 
  private:
   // Heterogenously compares modules by base address, and modules and
@@ -173,6 +177,9 @@
   std::set<std::unique_ptr<const Module>, ModuleAndAddressCompare>
       native_modules_;
 
+  // Lock to guard |non_native_modules_|.
+  mutable base::Lock lock_;
+
   // Set of non-native modules currently mapped into the address space, sorted
   // by base address. Represented as flat_set because std::set does not support
   // extracting move-only element types prior to C++17's
@@ -182,7 +189,7 @@
   // native_modules_ to support preferential lookup of non-native modules
   // embedded in native modules; see comment on UpdateNonNativeModules().
   base::flat_set<std::unique_ptr<const Module>, ModuleAndAddressCompare>
-      non_native_modules_;
+      non_native_modules_ GUARDED_BY(lock_);
 
   // Unsorted vector of inactive non-native modules. Inactive modules are no
   // longer mapped in the address space and don't participate in address lookup,