Skip to content

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Oct 22, 2025

Address a bug while investigating #1371

If write action client code like below,

            while rclpy.ok:
                future = action_client.send_goal(xxx)
                rclpy.spin_until_future_complete(action_client, future)

In spin_until_future_complete(), a callback is added to the done callback of future. And next spin_once_until_future_complete() will be called continuously.

def spin_until_future_complete(
self,
future: Future,
timeout_sec: Optional[float] = None
) -> None:
"""Execute callbacks until a given future is done or a timeout occurs."""
# Make sure the future wakes this executor when it is done
future.add_done_callback(lambda x: self.wake())
if timeout_sec is None or timeout_sec < 0:
while self._context.ok() and not future.done() and not self._is_shutdown:
self.spin_once_until_future_complete(future, timeout_sec)
else:
start = time.monotonic()
end = start + timeout_sec
timeout_left = TimeoutObject(timeout_sec)
while self._context.ok() and not future.done() and not self._is_shutdown:
self.spin_once_until_future_complete(future, timeout_left)

Each time spin_once_until_future_complete() is called, the same callback is added to the future's done callback.

def spin_once_until_future_complete(
self,
future: Future,
timeout_sec: Optional[Union[float, TimeoutObject]] = None
) -> None:
future.add_done_callback(lambda x: self.wake())
self._spin_once_impl(timeout_sec, future.done)

This way, the future's done callbacks keep accumulating.

When a future is completed (calls set_result()), _schedule_or_invoke_done_callbacks() is called.
In this function, it creates a Task for each done callback, which leads to many unnecessary Tasks being created and consuming more and more memory.

rclpy/rclpy/rclpy/task.py

Lines 150 to 153 in a09a031

if executor is not None:
# Have the executor take care of the callbacks
for callback in callbacks:
executor.create_task(callback, self)

So this PR is to remove the code that adds unnecessary done callbacks.


This is an automatic backport of pull request #1374 done by Mergify.

… spin_until_future_complete (#1374)

Signed-off-by: Barry Xu <[email protected]>
(cherry picked from commit c009b0d)

# Conflicts:
#	rclpy/rclpy/executors.py
@mergify mergify bot added the conflicts label Oct 22, 2025
@mergify
Copy link
Contributor Author

mergify bot commented Oct 22, 2025

Cherry-pick of c009b0d has failed:

On branch mergify/bp/jazzy/pr-1374
Your branch is up to date with 'origin/jazzy'.

You are currently cherry-picking commit c009b0d.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   rclpy/rclpy/executors.py

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Signed-off-by: Tomoya Fujita <[email protected]>
@fujitatomoya fujitatomoya self-assigned this Oct 22, 2025
@fujitatomoya
Copy link
Collaborator

@Barry-Xu-2018 i resolved the conflicts for this backport, can you review this?

@fujitatomoya
Copy link
Collaborator

Pulls: #1530
Gist: https://gist.githubusercontent.com/fujitatomoya/646eb7096d17b3006487ef22aa826266/raw/ca0501a18b6c548f80f0a7061d6ade8fbe79fc5a/ros2.repos
BUILD args: --packages-above-and-dependencies ‎rclpy
TEST args: --packages-above ‎rclpy
ROS Distro: jazzy
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/17336

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Copy link
Contributor

@Barry-Xu-2018 Barry-Xu-2018 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Barry-Xu-2018
Copy link
Contributor

Rerun CI

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants