Repository navigation
test: fix races around executor cancel, report waits and peer ports - #722
Merged
Merged
Conversation
bburda
added this pull request to stack #728
October 18, 2026 17:38
4 of 7 tasks
bburda
force-pushed
the
test/close-test-races
branch
from
October 19, 2026 07:00
f29a36e to
06ff89e
Compare
mfaferek93
reviewed
Oct 19, 2026
bburda
force-pushed
the
test/close-test-races
branch
from
October 20, 2026 10:54
06ff89e to
61aee78
Compare
Executor::cancel() only clears the spinning flag, and spin() sets it again when it starts. A cancel() made before spin() starts is lost. spin() then never returns and join() blocks until the ctest timeout. - Tests that spin an executor on their own thread wait for is_spinning() before they can cancel it. - The shared demo node and the ghost node injector set a stop flag and spin in 100 ms steps until it is set. A SIGTERM that arrives while a demo node is still being built is kept, so launch does not escalate to SIGKILL. Other races: - The node_death integration cases call flush_reports() after a detector tick. It sends a marker request on the same client and waits for the answer. The fake fault manager serves requests in arrival order, so every earlier report is recorded by then. - test_grouping_entity_aggregation waits for the peer operation it runs before it sends it. - test_06 of test_graph_leftover_nodes sends the late sample only after GET /apps stops listing the nodes, and checks after the sample is published and acknowledged. The leftover fixture gets wait_published(). - The forwarding, peer client, aggregation manager and OPC UA tests pointed their unreachable peer at fixed ports inside the kernel's ephemeral range, which any process can hold. test_support::RefusedPort binds a loopback socket to a kernel-chosen port and never listens. A connect to it is refused at once, and no other socket can bind the port while it lives. - The healing_threshold case of test_lifecycle_expectation_e2e waited a fixed 2 s after each blink for a fault frame. The detector re-sends GRAPH_NODE_INACTIVE only once the replacement process's lifecycle label is measured, and under load its GetState can stay unanswered for several ticks. The case now waits for the frame as long as the watcher's GetState schedule for a respawned node. - The baseline cases of test_graph_provider_stale and test_graph_provider_greenwave read pipeline_status from the first graph that had a frequency. greenwave's rate is a rolling estimate, and after a short publisher stall its first sample can sit below the degraded ratio. These cases now wait for a healthy pipeline, as the recovery case does. - test_updates waited up to 10 s in every cleanup, also for a package that was never prepared, and ran close to its 120 s timeout. The cleanup now waits only while an operation runs or a DELETE is in flight. Warnings: - Two gateway tests read files through rdbuf(). GCC 13 reports -Wnull-dereference inside libstdc++ for a std::string built from istreambuf_iterator in the sanitizer builds. - clang-tidy findings are fixed in test_fault_manager, test_ros2_subscription_slot, test_graph_watchdog_plugin, test_opcua_plugin and the unresponsive parameter node. No behaviour change. tsan_suppressions.txt gains the action send_goal path. The executor thread frees the shared SendGoal response in uninstrumented librclcpp, the same handoff as the call_service entry.
bburda
force-pushed
the
test/close-test-races
branch
from
October 20, 2026 12:55
61aee78 to
efba269
Compare
mfaferek93
approved these changes
Oct 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Fixes test races that time out or fail under parallel load, and the warnings in the tests this touches.
is_spinning()first. Acancel()made beforespin()starts is lost, andjoin()then blocks until the ctest timeout.test_grouping_entity_aggregationwaits for the peer operation it runs. test_06 oftest_graph_leftover_nodessends its late sample after the gateway read the departure.test_support::RefusedPort, a bound loopback socket that never listens. The fixed ports used before are in the ephemeral range, where any process can hold them.test_lifecycle_expectation_e2ewaits for the next fault frame after each blink as long as the watcher's GetState schedule for a respawned node, plus one tick for the watcher to see the respawn and one more failed read. It waited a fixed 2 s, and under load the replacement's lifecycle label was not measured yet, so no frame came.test_graph_provider_staleandtest_graph_provider_greenwavewait for a healthy pipeline. greenwave's rate is a rolling estimate, and after a short publisher stall its first sample can sit below the degraded ratio.test_updateswaits in cleanup only while an operation runs.rdbuf(), which removes GCC 13-Wnull-dereferencewarnings in the sanitizer builds. clang-tidy findings in the touched tests are fixed.tsan_suppressions.txtgains the actionsend_goalpath: the executor thread frees the shared SendGoal response in uninstrumented librclcpp, the same handoff as thecall_serviceentry.Issue
Type
Testing
./scripts/test.sh unit --packages-select ros2_medkit_gateway ros2_medkit_fault_manager ros2_medkit_graph_watchdog ros2_medkit_opcuaand./scripts/test.sh integ --packages-select ros2_medkit_integration_tests ros2_medkit_graph_watchdog: everything passes.spin()in the spin thread oftest_discovery_manager. Without the change the test hangs until its timeout, with it the test passes.Checklist