fix: Prevent client destruction race in services.call_service (backport #1255)

[mergify]

Summary

services.call_service and AdvertisedActionHandler both create rclpy entities (rclpy.Client and rclpy.action.ActionServer, respectively) on the rosbridge node directly from a worker thread, while the rclpy executor is concurrently spinning that same node. The executor's wait-set rebuild can race with the worker thread's entity registration / destruction.

The symptom varies by entity:

• Service clients silently lose their backing handle: the executor ends up holding a Client whose PyCapsule has been freed, and the next rclpy binding call on the affected node raises TypeError: Object of type 'NoneType' is not an instance of 'capsule'. After this trips once, every subsequent call_service on the affected websocket returns result: false and the connection is unrecoverable until the rosbridge process restarts.

• Action servers crash harder: the worker thread reaches _rclpy.ActionServer(...) in rclpy/action/server.py:__init__ while the executor is mid-wait-set-rebuild and the process SIGSEGVs. This is the actual cause of the test_capabilities hang on Rolling/Lyrical CI — pytest's xunit fires after the segfault, but ctest waits the full 60 s before declaring the test timed out, masking the real signal in run logs.

Approach

Same idea as the IncomingQueue race fixed in #1183: route the lifecycle calls through the executor via executor.create_task(...) so they serialize with the executor's own work instead of competing with it.

Extract the executor-routing pattern into rosbridge_library.internal.executor_helpers — one module, two helpers:

Helper	Returns	Used for
run_on_executor(node, fn)	fn()'s return value	Entity creation, where the caller needs the new handle before continuing
schedule_on_executor(node, fn)	nothing (fire-and-forget)	Entity destruction, where the caller just needs the teardown to happen on the right thread

Both fall back to inline invocation when the node has no attached executor, which keeps unit-test setups (no executor) working unchanged.

What changed

• services.call_service now routes create_client through run_on_executor (it needs the resulting Client before it can call wait_for_service / call_async). The three destroy_client sites — wait-for-service failure, response timeout, and the successful path — go through schedule_on_executor.
• AdvertisedActionHandler.__init__ routes ActionServer(...) through run_on_executor. (ActionServer destruction was already routed through the executor in feat: Improve action unadvertising #1248, so only the creation side needed the fix.)
• ActionTester in test/internal/actions/test_actions.py builds its own ActionServer from the test thread while the fixture's executor is spinning. The same SIGSEGV reproduced there, so its ActionServer(...) call now goes through run_on_executor too.
• Scope is intentionally limited to the create/destroy lifecycle. The read-only setup calls in call_service (resolve_service_name, get_service_names_and_types) stay on the worker thread.

Regression test

rosbridge_library/test/internal/services/test_stress_service_clients.py exercises the race window:

• test_many_parallel_service_calls — 40 ServiceCallers in flight at once against the same service.
• test_barrier_synchronized_service_calls — 20 callers start at the same instant via a threading.Barrier.
• test_repeated_rapid_service_calls — 100 sequential calls with no settle time between iterations.
• test_interleaved_waves_of_service_calls — each wave of callers starts while the previous wave is still tearing down its clients.

After each scenario, the test asserts the executor's spin thread is still alive, has logged no errors, and can still process newly submitted tasks. The action-server race is covered by the existing test_action_capabilities and test_actions tests, which previously SIGSEGV'd and now pass.

Back-ports

services.py and advertise_action.py are identical on jazzy and humble; sibling PRs target those branches. Happy to defer those to mergify if preferred.

---

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

[mergify]

Cherry-pick of 06bf324 has failed:

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

You are currently cherry-picking commit 06bf324.
  (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)

Changes to be committed:
	new file:   rosbridge_library/src/rosbridge_library/internal/executor_helpers.py
	modified:   rosbridge_library/src/rosbridge_library/internal/services.py
	modified:   rosbridge_library/test/capabilities/test_action_capabilities.py
	new file:   rosbridge_library/test/internal/services/test_stress_service_clients.py

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   rosbridge_library/src/rosbridge_library/capabilities/advertise_action.py
	both modified:   rosbridge_library/test/internal/actions/test_actions.py

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

[bjsowa]

@Mergifyio backport jazzy

[mergify]

backport jazzy

✅ Backports have been created

Details

• #1270 fix: Prevent client destruction race in services.call_service (backport #1255) (backport #1269) has been created for branch jazzy but encountered conflicts

Cherry-pick of 7631e3f has failed:

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

You are currently cherry-picking commit 7631e3f.
  (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)

Changes to be committed:
	new file:   rosbridge_library/src/rosbridge_library/internal/executor_helpers.py
	modified:   rosbridge_library/src/rosbridge_library/internal/services.py
	modified:   rosbridge_library/test/capabilities/test_action_capabilities.py
	new file:   rosbridge_library/test/internal/services/test_stress_service_clients.py

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   rosbridge_library/src/rosbridge_library/capabilities/advertise_action.py
	both modified:   rosbridge_library/test/internal/actions/test_actions.py

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