Fix Blackboard thread-safety bugs and XML null pointer dereference #1116
+1,163
−51
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
New control node that executes a try-child and, on failure, runs a catch-child as recovery. Supports configurable catch_on_halt behavior, async children, and re-entry after RUNNING states. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add null check for subtree_id when <SubTree> in <TreeNodesModel> is missing the ID attribute — now throws descriptive RuntimeError instead of crashing with UB (null pointer as map key) - Fix variable name shadowing: rename inner-loop 'name' to 'port_name' to avoid shadowing the outer structured binding Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
facontidavide
force-pushed
the
bug_fixes_auto
branch
from
cb93feb to
2b9e571
Compare
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>
facontidavide
force-pushed
the
bug_fixes_auto
branch
from
2b9e571 to
519917d
Compare
Switch storage_mutex_ from std::mutex to std::shared_mutex so that concurrent readers (getEntry, getKeys, debugMessage, set initial lookup, cloneInto snapshot) no longer block each other. Write operations (createEntryImpl, unset, clear, cloneInto insert/erase) still take exclusive locks. Also remove the dead entry_mutex_ recursive_mutex and its deprecated entryMutex() accessor — no callers exist in the codebase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
set()new-entry path: missingentry_mutexlock when writing value/sequence_id/stampset()existing-entry path: use-after-free risk from raw reference afterstorage_mutex_unlockcloneInto(): missing per-entry locks when copying entry members + lock-order inversion (potential deadlock) betweenstorage_mutex_andentry_mutex— restructured into 3-phase approach that never holds both locks simultaneouslyImportBlackboardFromJSON(): unprotected write toentry->valuedebugMessage(): iteratesstorage_withoutstorage_mutex_getKeys(): iteratesstorage_withoutstorage_mutex_loadSubtreeModelnull pointer dereference when<SubTree>in<TreeNodesModel>is missing theIDattributeloadSubtreeModelTest plan
RuntimeErrorinstead of crashing🤖 Generated with Claude Code