[rclpy] Fix spin_once() removing node from executor even if already attached

[alonborn]

Summary

This pull request fixes a bug where rclpy.spin_once() would remove a node from its executor that it didn't add.

Previously, spin_once() unconditionally called executor.remove_node(node) at the end, disconnecting nodes managed by user-created executors (e.g., MultiThreadedExecutor).

This fix tracks whether spin_once() itself added the node, and only removes it if necessary — matching the behavior already used in spin_until_future_complete().

---

Motivation

Calling rclpy.spin_once() on a node that is part of an existing executor (e.g., for asynchronous callbacks) currently breaks the executor's management, causing nodes to silently stop working.

This fix ensures better safety and makes rclpy utilities more consistent.

---

Related Issue

It handles a similar issue like #1316

---

[alonborn]

Signed-off-by added via rebase. Ready for review!

[Barry-Xu-2018]

Pulls: #1446
Gist: https://gist.githubusercontent.com/Barry-Xu-2018/b7bf6ed7353257611b68a0995631452b/raw/6dfbd73453f649e5cc3425271d360336ec7ccd89/ros2_rolling.repos
BUILD args: --packages-above-and-dependencies rclpy
TEST args: --packages-above rclpy
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15807

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[fujitatomoya]

i am not opposed to have this fix in general, because we already have the similar fix with #1316.

But I do not think this PR fixes the issue reported on #1445, because MultiThreadedExecutor is still going to be removed in ths 1st spin_once. IMO, this PR or #1316 only works if the application uses get_global_executor to avoid removing the executor.

Note: i mean that the application needs to add executor argument to spin_once.

can you explain what problem are you trying to address here?

[alonborn]

Hi @fujitatomoya,
You're absolutely right — this is similar to #1316, but it does not resolve #1445. I've updated the comment above to clarify that.

I originally encountered this unexpected behavior in pymoveit, where calling spin_once caused the executor to lose its node. This fix, combined with explicitly passing the executor to spin_once, allows it to function as expected in that context.

Hope this clears things up — please let me know if anything else is needed.

Thanks again,
Alon

[fujitatomoya]

@alonborn thanks for the comment.

yeah, for doing resolve the original issue, we need to use executor optional argument with this PR.
anyway this PR does make sense to support that case.

[fujitatomoya]

Note

to keep the consistency with #1316, we need to backport this to all downstream distros.

[fujitatomoya]

@Mergifyio backport kilted jazzy humble

[mergify]

backport kilted jazzy humble

✅ Backports have been created

• #1449 [rclpy] Fix spin_once() removing node from executor even if already attached (backport #1446) has been created for branch kilted
• #1450 [rclpy] Fix spin_once() removing node from executor even if already attached (backport #1446) has been created for branch jazzy
• #1451 [rclpy] Fix spin_once() removing node from executor even if already attached (backport #1446) has been created for branch humble