update guard condition by managing multiple items#527
update guard condition by managing multiple items#527iuhilnehc-ynos wants to merge 2 commits intoros2:masterfrom
Conversation
32c5c09 to
4e4ab1b
Compare
…ables Signed-off-by: Chen Lihui <lihui.chen@sony.com>
4e4ab1b to
33ea304
Compare
| std::atomic_bool hasTriggered_; | ||
| std::mutex * conditionMutex_ RCPPUTILS_TSA_GUARDED_BY(internalMutex_); | ||
| std::condition_variable * conditionVariable_ RCPPUTILS_TSA_GUARDED_BY(internalMutex_); | ||
| std::list<std::pair<std::mutex *, std::condition_variable *>> conditions_ |
There was a problem hiding this comment.
I think we should make this a std::vector? The most common operations (in my opinion) will be trigger() which has to loop over this sequence of pairs. If removing a pair is expensive (inside of detachCondition()), I don't think that's as important.
There was a problem hiding this comment.
Thank you, I'll update it.
|
CI:
|
I noted this
From the comments behind, I think running this CI is intent to use
expected because of using
|
Co-authored-by: William Woodall <william@osrfoundation.org> Signed-off-by: Chen Lihui <lihui.chen@sony.com>
| // TODO(iuhilnehc-ynos): conditionMutex is not used, remove it | ||
| std::unique_lock<std::mutex> clock(*cond.first); | ||
| clock.unlock(); |
There was a problem hiding this comment.
Quick question, are these actually needed? just curious why not removing now?
|
not a correct fix, close it |
related to ros2/rclcpp#1611
Signed-off-by: Chen Lihui lihui.chen@sony.com