Skip to content

ci: take ROS packages from one sync, and shut rclcpp down after the param_drift suite - #692

Open
bburda wants to merge 2 commits into
mainfrom
ci/lyrical-consistent-ros-packages
Open

bburda wants to merge 2 commits into
mainfrom
ci/lyrical-consistent-ros-packages

Conversation

@bburda

@bburda bburda commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Since the ROS apt repository for lyrical was republished on 2026-09-18, build-and-test (lyrical) fails on every branch. The ros:lyrical-ros-base image still carries packages from an older sync. rosdep then installs example_interfaces and test_msgs from the current sync, which were built against a newer rosidl_typesupport_fastrtps_c. They call has_buffer_fields_* and cdr_*_with_endpoint_* functions that the image's service_msgs, builtin_interfaces and unique_identifier_msgs do not export, so every test that loads their FastRTPS type support exits with a symbol lookup error (for example test_operation_manager and the action demo nodes, exit code 127).

This PR runs apt-get upgrade -y in the dependency step of every job whose matrix includes lyrical (ci.yml build-and-test and graph-watchdog, opcua-plugin.yml unit-tests), so the whole package set comes from one sync. The Depends lines of these packages carry no versions, so apt does not pull the rebuilt dependencies on its own.

The runtime stage of the root Dockerfile has the same gap for the published lyrical image. It is not changed here.

The second commit fixes a crash that came with the same package update on jazzy (rclcpp 28.1.22). test_param_drift_integration could die with SIGSEGV after all its tests passed. The suite never shuts rclcpp down, so the default context runs Context::shutdown() from its static destructor inside exit(). Since 28.1.22 that call inserts the context into a thread_local set, and one case of the suite creates that set on the main thread by shutting its own context down. exit() destroys the set before the static destructors run. TearDownTestSuite() now calls rclcpp::shutdown(), so the default context is no longer valid when its destructor runs.


Issue

  • none, CI-only change

Type

  • Bug fix
  • New feature or tests
  • Breaking change
  • Documentation only

Testing

In a container from the current ros:lyrical-ros-base image with the job's install steps:

  • before: ldd -r reports unresolved symbols in the example_interfaces, test_msgs and greenwave_monitor_interfaces FastRTPS type support libraries, and test_operation_manager fails with the same symbol lookup error as CI;
  • after apt-get upgrade -y (130 packages, none held back or removed): ldd -r is clean on every /opt/ros/lyrical/lib/*.so and test_operation_manager passes 41 tests.

The same upgrade runs cleanly on the current humble and jazzy images.

In a ros:jazzy-ros-base container (rclcpp 28.1.22, CycloneDDS as in CI): the full param_drift suite crashed 4 of 50 runs before the change and 0 of 50 after; the case that shuts its own context down crashed 36 of 500 runs alone before and 0 of 500 after, and a case that never does crashed 0 of 500. colcon test --packages-select ros2_medkit_graph_watchdog: 725 test cases, 0 failures.

The CI run on this PR is the full check.


Checklist

  • Breaking changes are clearly described (and announced in docs / changelog if needed)
  • Tests were added or updated if needed
  • Docs were updated if behavior or public API changed

The ros:<distro>-ros-base images can lag the ROS apt repository. rosdep
then installs packages from the current sync next to image packages from
an older one, and the two sets do not always link. On lyrical,
example_interfaces and test_msgs from the current sync call
has_buffer_fields_* and cdr_*_with_endpoint_* functions that the image's
service_msgs, builtin_interfaces and unique_identifier_msgs do not
export, so every test that loads their FastRTPS type support exits with
a symbol lookup error.

Run apt-get upgrade in the dependency step of every job whose matrix
includes lyrical, so the whole package set comes from one sync.
…ion suite

test_param_drift_integration could die with SIGSEGV after all 52 tests had
passed. The suite initialises rclcpp and never shuts it down, so the default
context is shut down by its own static destructor inside exit().

Since rclcpp 28.1.22, Context::shutdown() first inserts the context into a
thread_local set that guards against reentrant calls. exit() destroys the main
thread's thread_local objects before it runs static destructors. The case
ContextShutdownStopsTheReaderWhileTheDetectorIsStillAlive shuts a context of
its own down on the main thread, which creates that set on this thread. At
exit the default context's destructor then inserts into the destroyed set and
reads its freed bucket array, which crashes whenever the freed memory holds a
non-null pointer at that slot.

TearDownTestSuite now calls rclcpp::shutdown(). The default context is no
longer valid when its destructor runs, and Context::shutdown() returns before
it reaches the set.
@bburda bburda changed the title ci: upgrade image packages before installing ROS dependencies ci: take ROS packages from one sync, and shut rclcpp down after the param_drift suite Sep 19, 2026
@bburda
bburda marked this pull request as ready for review September 19, 2026 15:35
@bburda bburda self-assigned this Sep 19, 2026
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.

1 participant