Fix Blackboard thread-safety data races

Fix 6 data races in Blackboard, verified with ThreadSanitizer (clang-21):

- set() new-entry path: wrote value/sequence_id/stamp without
  entry_mutex after createEntryImpl — add scoped_lock on entry_mutex
- set() existing-entry path: held raw reference after unlocking
  storage_mutex_, risking use-after-free if concurrent unset() erases
  the entry — copy shared_ptr before unlocking
- cloneInto(): read/wrote entry members without entry_mutex —
  add scoped_lock on src and dst entry mutexes
- ImportBlackboardFromJSON(): wrote entry->value without any lock —
  add scoped_lock on entry_mutex
- debugMessage(): iterated storage_ without storage_mutex_ —
  add unique_lock
- getKeys(): iterated storage_ without storage_mutex_ —
  add unique_lock

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: facontidavide

include/behaviortree_cpp/blackboard.h

@@ -300,8 +300,10 @@ inline void Blackboard::set(const std::string& key, const T& value)
                         GetAnyFromStringFunctor<T>());
       entry = createEntryImpl(key, new_port);
     }
-    storage_lock.lock();
 
+    // Lock entry_mutex before writing to prevent data races with
+    // concurrent readers (BUG-1/BUG-8 fix).
+    std::scoped_lock entry_lock(entry->entry_mutex);
     entry->value = new_value;
     entry->sequence_id++;
     entry->stamp = std::chrono::steady_clock::now().time_since_epoch();
@@ -310,8 +312,11 @@ inline void Blackboard::set(const std::string& key, const T& value)
   {
     // this is not the first time we set this entry, we need to check
     // if the type is the same or not.
-    Entry& entry = *it->second;
+    // Copy shared_ptr to prevent use-after-free if another thread
+    // calls unset() while we hold the reference (BUG-2 fix).
+    auto entry_ptr = it->second;
     storage_lock.unlock();
+    Entry& entry = *entry_ptr;
 
     std::scoped_lock scoped_lock(entry.entry_mutex);
 

src/blackboard.cpp

@@ -103,6 +103,9 @@ void Blackboard::addSubtreeRemapping(StringView internal, StringView external)
 
 void Blackboard::debugMessage() const
 {
+  // Lock storage_mutex_ to prevent iterator invalidation from
+  // concurrent modifications (BUG-5 fix).
+  const std::unique_lock<std::mutex> storage_lock(storage_mutex_);
   for(const auto& [key, entry] : storage_)
   {
     auto port_type = entry->info.type();
@@ -136,6 +139,9 @@ void Blackboard::debugMessage() const
 
 std::vector<StringView> Blackboard::getKeys() const
 {
+  // Lock storage_mutex_ to prevent iterator invalidation and
+  // dangling StringView from concurrent modifications (BUG-6 fix).
+  const std::unique_lock<std::mutex> storage_lock(storage_mutex_);
   if(storage_.empty())
   {
     return {};
@@ -200,8 +206,10 @@ void Blackboard::cloneInto(Blackboard& dst) const
     auto it = dst_storage.find(src_key);
     if(it != dst_storage.end())
     {
-      // overwrite
+      // overwrite — lock both entry mutexes to prevent data races
+      // with concurrent readers that hold shared_ptr<Entry> (BUG-3 fix).
       auto& dst_entry = it->second;
+      std::scoped_lock entry_locks(src_entry->entry_mutex, dst_entry->entry_mutex);
       dst_entry->string_converter = src_entry->string_converter;
       dst_entry->value = src_entry->value;
       dst_entry->info = src_entry->info;
@@ -210,7 +218,8 @@ void Blackboard::cloneInto(Blackboard& dst) const
     }
     else
     {
-      // create new
+      // create new — lock src entry to read its members safely
+      std::scoped_lock src_lock(src_entry->entry_mutex);
       auto new_entry = std::make_shared<Entry>(src_entry->info);
       new_entry->value = src_entry->value;
       new_entry->string_converter = src_entry->string_converter;
@@ -317,6 +326,8 @@ void ImportBlackboardFromJSON(const nlohmann::json& json, Blackboard& blackboard
         blackboard.createEntry(it.key(), res->second);
         entry = blackboard.getEntry(it.key());
       }
+      // Lock entry_mutex before writing to prevent data races (BUG-4 fix).
+      std::scoped_lock lk(entry->entry_mutex);
       entry->value = res->first;
     }
   }

tests/CMakeLists.txt

@@ -55,6 +55,7 @@ set(BT_TESTS
   gtest_simple_string.cpp
   gtest_polymorphic_ports.cpp
   gtest_plugin_issue953.cpp
+  gtest_blackboard_thread_safety.cpp
   gtest_xml_null_subtree_id.cpp
 
   script_parser_test.cpp