[Blackboard] Deadlock on concurrent set() and getInput().

[FabianSchurig]

Hello @facontidavide,

We found the following deadlock using behaviortree_cpp version 3.6.1.
It deadlocks in the following case using the newly introduced entry_mutex_ from 06a1f4c.

Lets think about the following two concurrent threads:

Thread A calls Blackboard::set()
Thread A locks mutex_
Thread B calls TreeNode::getInput()
Thread B locks entry_mutex_
Thread A waits for entry_mutex_
Thread B waits for mutex_

The order of locking introduces the deadlock.

Mitigation:
In Blackboard::set() first lock entry_mutex_ then lock mutex_.

Thread A calls Blackboard::set()
Thread A locks entry_mutex_
Thread B calls TreeNode::getInput()
Thread B waits for entry_mutex_
Thread A locks mutex_
Thread A releases mutex_ and entry_mutex_
Thread B locks entry_mutex_
Thread B locks mutex_
Thread B releases mutex_ and entry_mutex_

Thanks in advance!

[FabianSchurig]

Suggestion 2:

Remove the entry_mutex_ and then lock the following also with the mutex_.

https://github.com/BehaviorTree/BehaviorTree.CPP/blob/master/include/behaviortree_cpp_v3/blackboard.h#L90-L94
https://github.com/BehaviorTree/BehaviorTree.CPP/blob/master/include/behaviortree_cpp_v3/blackboard.h#L104-L111

Then the blackboard interface is clean to its public API and any user of the Blackboard does not need to think about threading/data race issues.

[facontidavide]

i think I like suggestion 2 more. Let me think about it.

[jediofgever]

I think I encountered this when I updated to new Foxy distributed packages. Is there a way to include the fix into binary of behaviortree_cpp_v3 ?