fix use-after-free from dangling StringView in Blackboard::getKeys

include/behaviortree_cpp/blackboard.h

@@ -123,7 +123,7 @@ class Blackboard
 
   void debugMessage() const;
 
-  [[nodiscard]] std::vector<StringView> getKeys() const;
+  [[nodiscard]] std::vector<std::string> getKeys() const;
 
   [[deprecated("This command is unsafe. Consider using Backup/Restore instead")]] void
   clear();

src/blackboard.cpp

@@ -137,16 +137,14 @@ void Blackboard::debugMessage() const
   }
 }
 
-std::vector<StringView> Blackboard::getKeys() const
+std::vector<std::string> Blackboard::getKeys() const
 {
-  // Lock storage_mutex_ (shared) to prevent iterator invalidation and
-  // dangling StringView from concurrent modifications (BUG-6 fix).
+  // Return owning copies of the keys, made while holding the shared lock.
+  // Returning StringView into storage_ would dangle as soon as another thread
+  // erased an entry (Blackboard::unset / the UnsetBlackboard action) after the
+  // lock is released but before the caller reads the view.
   const std::shared_lock storage_lock(storage_mutex_);
-  if(storage_.empty())
-  {
-    return {};
-  }
-  std::vector<StringView> out;
+  std::vector<std::string> out;
   out.reserve(storage_.size());
   for(const auto& entry_it : storage_)
   {
@@ -337,9 +335,8 @@ std::shared_ptr<Blackboard::Entry> Blackboard::createEntryImpl(const std::string
 nlohmann::json ExportBlackboardToJSON(const Blackboard& blackboard)
 {
   nlohmann::json dest;
-  for(auto entry_name : blackboard.getKeys())
+  for(const auto& name : blackboard.getKeys())
   {
-    const std::string name(entry_name);
     if(auto any_ref = blackboard.getAnyLocked(name))
     {
       if(auto any_ptr = any_ref.get())

tests/gtest_blackboard_thread_safety.cpp

@@ -335,3 +335,48 @@ TEST(BlackboardThreadSafety, GetKeysWhileModifying_Bug6)
 
   SUCCEED();
 }
+
+// BUG-6 (continued): getKeys() used to return StringView pointing into the
+// std::string keys of storage_. The shared lock is released when getKeys()
+// returns, so a concurrent erase (unset() / the UnsetBlackboard action) frees
+// the underlying key while the caller still reads the view. This is exactly
+// what ExportBlackboardToJSON does on the Groot2 server thread.
+// Under ASan/TSan this reports a heap-use-after-free before the fix.
+TEST(BlackboardThreadSafety, GetKeysReadWhileErasing_Bug6)
+{
+  auto bb = Blackboard::create();
+
+  constexpr int kIterations = 200000;
+  std::atomic<bool> stop{ false };
+
+  auto reader = [&]() {
+    while(!stop.load())
+    {
+      for(const auto& key : bb->getKeys())
+      {
+        // Read the key content after getKeys() released the lock.
+        volatile size_t s = std::string(key).size();
+        (void)s;
+      }
+    }
+  };
+
+  auto mutator = [&]() {
+    for(int i = 0; i < kIterations; i++)
+    {
+      const std::string key = "key_" + std::to_string(i % 64);
+      bb->set(key, i);
+      bb->unset(key);  // erase -> frees the map key's std::string
+    }
+    stop.store(true);
+  };
+
+  std::thread t1(reader);
+  std::thread t2(mutator);
+
+  t2.join();
+  stop.store(true);
+  t1.join();
+
+  SUCCEED();
+}