fix: Fixed expiring of goals if events executor is used #2674
Conversation
jmachowinski
requested review from
ivanpauno,
hidmic and
wjwwood
as code owners
|
I don't think creating a new thread per |
|
@mauropasse Note, this is a backport to jazzy. The timer interface is not exposed, therefore there is no way to register a timer at the executor without breaking ABI/API. |
|
This pull request has been mentioned on ROS Discourse. There might be relevant details there: https://discourse.ros.org/t/next-client-library-wg-meeting-friday-22nd-november-2024/40703/1 |
|
@jmachowinski just checking that this is jazzy of #2699 (rolling) to keep the ABI/API, right? |
|
@fujitatomoya yes, this is the jazzy version, as the rolling version was merged, I'll try to polish it within the next days... |
|
@jmachowinski sounds great! ping me whenever it is ready, happy to review. (trading reviews 👍 ) |
jmachowinski
force-pushed
the
action_leak_fix_jazzy
branch
from
6ea3c59 to
f51a729
Compare
|
@fujitatomoya ready for review. Note, this builds up on top of the code from #2691 |
jmachowinski
force-pushed
the
action_leak_fix_jazzy
branch
from
f51a729 to
7f2ad63
Compare
Added two new synchronization primitives to perform waits using an rclcpp::Clock. Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
This commit is only relevant if the events executor is used. This commit starts a background thread, to create events for the expire timer of the action. Without this the action server will not expire old goal results leading to memory and performance issues. Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
jmachowinski
force-pushed
the
action_leak_fix_jazzy
branch
from
7f2ad63 to
805700c
Compare
This commit is only relevant if the events executor is used.
This commit adds a background thread, to create events for the expire timer of the action.
Without this the action server will not expire old goal results leading to memory and performance issues.
For now, this commit is a draft. There is still a (minor) issue, that the clock used in the background
thread does not work correct in the use_sim_time case (it still runs on system time).
This is an alternative to #2661
In contrast to #2661, it reuses the rcl internal timer and therefore existing code to recompute the
expire times.
@alsora @mauropasse @mjcarroll thoughts ?