Skip to content

Release ROS adapter resources during shutdown - #566

Open
ktyang512 wants to merge 1 commit into
ros2:rollingfrom
ktyang512:fix-565-ros-adapter-resources
Open

Release ROS adapter resources during shutdown#566
ktyang512 wants to merge 1 commit into
ros2:rollingfrom
ktyang512:fix-565-ros-adapter-resources

Conversation

@ktyang512

Copy link
Copy Markdown

Description

Fixes #565.

ROSAdapter.shutdown() stopped its executor thread and shut down its node and
context, but it retained the node wrapper and did not shut down the executor
itself. In repeated launch tests, the retained rclpy/Fast DDS resources were
only released by delayed cyclic garbage collection, so file descriptors and DDS
threads accumulated until another file or pipe creation failed with EMFILE.

This change explicitly shuts down the executor and releases the adapter's node
and private thread references after shutdown. It also adds a regression
covering executor cleanup, node release, and restart with fresh resources.

Is this user-facing behavior change?

After ROSAdapter.shutdown(), its ros_node property now returns None
instead of a destroyed node. The stopped context and executor remain
inspectable, and calling start() replaces all three with fresh objects.

Did you use Generative AI?

Yes. OpenAI Codex (GPT-5) assisted with candidate research, reproduction,
diagnosis, implementation, test design, and drafting. I reviewed the final diff
and validation evidence before submission.

Additional Information

Local validation on Rolling with Fast DDS:

  • the focused regression fails against the previous implementation and passes
    with this change;
  • all 5 launch_ros/test tests pass, including copyright, flake8, and pep257;
  • all 22 YAML/XML and executor-mode cases in
    test_launch_container_executor_modes pass under ulimit -n 64 in 118.59
    seconds, with file descriptors and threads stable at 11 and 8 after every
    case;
  • git diff --check passes.

Not tested locally: the full multi-package repository suite, ROS buildfarm
jobs, Windows, aarch64, or non-Fast-DDS RMW implementations.

Signed-off-by: ktyang512 <ktyang512@gmail.com>
@mergify

mergify Bot commented Aug 21, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🧑‍🌾 test_launch_ros: Intermittent test failures due to "Too many open files"

1 participant