BTRuntime::load() contracts to return LoadOutcome{ok, error} and preserve the previously-active definition on failure. It can instead throw, after having already committed the new definition.
The try/catch in load() wraps only createTreeFromFile. The subsequent populate_blackboard(*tree_) call sits outside it, and after tree_, tree_path_, loaded_def_bytes_, version_, and tree_loaded_ = true have all been assigned.
populate_blackboard does blackboard->set(kBTServiceContextKey, services). A tree that remaps a port onto anolis.bt_service_context declares that key with a different type at build time, so the set throws BT::LogicError. The tree builds fine — the collision only appears when the runtime writes the service context.
Result: load() throws instead of returning, and the runtime is left with tree_loaded_ = true and a definition that cannot be ticked. The caller's error path never runs and the previous good definition is gone.
Reproduced during review of #279 with:
<GetParameter param="x" value="{anolis.bt_service_context}"/>Fix is to move populate_blackboard inside the guarded region, or to stage it before the commit so a failure preserves the previous definition like every other load failure does.
Found while reviewing #280; not introduced by it.
BTRuntime::load()contracts to returnLoadOutcome{ok, error}and preserve the previously-active definition on failure. It can instead throw, after having already committed the new definition.The try/catch in
load()wraps onlycreateTreeFromFile. The subsequentpopulate_blackboard(*tree_)call sits outside it, and aftertree_,tree_path_,loaded_def_bytes_,version_, andtree_loaded_ = truehave all been assigned.populate_blackboarddoesblackboard->set(kBTServiceContextKey, services). A tree that remaps a port ontoanolis.bt_service_contextdeclares that key with a different type at build time, so thesetthrowsBT::LogicError. The tree builds fine — the collision only appears when the runtime writes the service context.Result:
load()throws instead of returning, and the runtime is left withtree_loaded_ = trueand a definition that cannot be ticked. The caller's error path never runs and the previous good definition is gone.Reproduced during review of #279 with:
Fix is to move
populate_blackboardinside the guarded region, or to stage it before the commit so a failure preserves the previous definition like every other load failure does.Found while reviewing #280; not introduced by it.