update guard condition by managing multiple items #527
Conversation
iuhilnehc-ynos
force-pushed
the
topic-guardcondition-manage-cond-list
branch
from
32c5c09
to
4e4ab1b
Compare
…ables Signed-off-by: Chen Lihui <[email protected]>
iuhilnehc-ynos
force-pushed
the
topic-guardcondition-manage-cond-list
branch
from
4e4ab1b
to
33ea304
Compare
| @@ -80,8 +82,8 @@ class GuardCondition | |||
| private: | |||
| std::mutex internalMutex_; | |||
| 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 <[email protected]> Signed-off-by: Chen Lihui <[email protected]>
| // 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 [email protected]