fix @ root remapping in Blackboard::createEntryImpl - #1192
Conversation
fallenmi
left a comment
There was a problem hiding this comment.
Reviewed exact head 5ab8b2adc60e046f3d767e82b4a4412e96417383 against current master c88a9f429a421b312599a07fa8902524b09bf90a.
I exercised the public restore path with a child param -> @shared remap. The same standalone reproducer exits 139 on the base and 0 on this head, and BlackboardTest.RemapToRootBlackboard passes locally. The implementation also applies the root redirect before taking the target blackboard's storage lock, matching getEntry, createEntry, and set semantics without introducing a recursive root lock.
The existing Windows pixi failure is in unrelated RepeatTestAsync.RepeatTestAsync; the changed blackboard test is green in the upstream build jobs.
Review performed with Codex assistance; I verified the current source and policy, exact SHAs, local RED/GREEN behavior, and CI logs.
A subtree port that remaps to a root blackboard key, written as port="{@foo}" in the XML, is stored as the remapping param -> @foo. When that entry is first created, createEntryImpl walks the remapping up to the parent but never applies the @ redirection that getEntry, set, and createEntry all use, so it lands as a literal @foo entry in the root storage. getEntry then strips the @ and looks up foo in the root, which never matches, so the two disagree about where the entry actually lives. I ran into it through ImportBlackboardFromJSON, the public restore path behind ImportTreeFromJSON: it looks the key up, misses, creates it, looks again, still gets null, and then locks entry_mutex on that null pointer for a segfault. The same split loses data silently as well, since set("param") writes into @foo while a reader gets foo from the root. Moving the @ handling getEntry already has into createEntryImpl keeps creation and lookup in agreement, and the added test crashes before the change and passes after.