From 91d23cac70cd9516e01db7f283bf3315d76a4c75 Mon Sep 17 00:00:00 2001 From: AlexTemirov Date: Mon, 10 Aug 2026 14:06:27 -0700 Subject: [PATCH] Scope managed ROS 2 process shutdown --- README.md | 3 +++ blacknode-package.toml | 2 +- components/processes/README.md | 6 ++++++ nodes/ros2.py | 5 ++--- nodes/ros2_runtime.py | 35 +++++++++++++++++++++++----------- tests/test_ros2_nodes.py | 25 +++++++++++++++++++----- 6 files changed, 56 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 38afd88..4573bab 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,9 @@ blacknode packages setup blacknode-ros2 - Stale detection, joint state, faults, stop, or shutdown suppress commands. - Rosbridge has no Blacknode pairing authentication; expose it only on a trusted network. - Managed subscriptions, processes, and streams have explicit stop paths. +- Native managed processes are stopped by their owned process handles. Host + command-pattern matching is not used, so an identical vendor `ros2 launch` + process is never selected for shutdown. ```powershell python -m pytest packages/blacknode-ros2/tests diff --git a/blacknode-package.toml b/blacknode-package.toml index ca6098c..5828b89 100644 --- a/blacknode-package.toml +++ b/blacknode-package.toml @@ -1,6 +1,6 @@ [package] name = "blacknode-ros2" -version = "0.6.1" +version = "0.6.2" description = "ROS 2 graph, topic, service, process, native, and rosbridge integration primitives." requires-blacknode = ">=0.3.0" layer = "ros2" diff --git a/components/processes/README.md b/components/processes/README.md index 3e54769..f83e7ac 100644 --- a/components/processes/README.md +++ b/components/processes/README.md @@ -4,5 +4,11 @@ Local colcon workspace builds plus managed `ros2 run` and `ros2 launch` process primitives. Connect `ROS2WorkspaceBuild.workspace_path` to the matching input on `ROS2Run` or `ROS2Launch` to run a package from that built overlay. +Native launches are session-scoped. Blacknode stops only the child process it +started and never uses command-pattern matching against the host ROS graph. +Existing vendor bringup processes, boot services, workspaces, and configuration +remain unchanged. Runtime restart or device reboot does not resume a stopped +workflow automatically. + This component depends on `core` and owns the node registrations under `components/processes/nodes`. diff --git a/nodes/ros2.py b/nodes/ros2.py index 3785be7..d374789 100644 --- a/nodes/ros2.py +++ b/nodes/ros2.py @@ -1110,8 +1110,7 @@ def ros2_launch(ctx: dict) -> dict: workspace_path = str(ctx.get("workspace_path") or "").strip() if action == "stop": - pattern = str(ctx.get("stop_pattern") or "").strip() or f"ros2 launch {package}".strip() or "ros2 launch" - result = rt.stop_ros2_managed(run_id, pattern=pattern) + result = rt.stop_ros2_managed(run_id) if result["ok"]: return { "launched": False, @@ -1213,7 +1212,7 @@ def ros2_run(ctx: dict) -> dict: pattern = " ".join(part for part in ("ros2", "run", package, executable) if part) if action == "stop": - result = rt.stop_ros2_managed(run_id, pattern=pattern or "ros2 run") + result = rt.stop_ros2_managed(run_id) if result.get("ok"): return { "running": False, diff --git a/nodes/ros2_runtime.py b/nodes/ros2_runtime.py index 09c2685..e5c00eb 100644 --- a/nodes/ros2_runtime.py +++ b/nodes/ros2_runtime.py @@ -886,7 +886,13 @@ def run_ros2_managed( workspace_path: str = "", ) -> dict[str, Any]: """Start one named background ``ros2 `` process, optionally in an overlay.""" - stop_ros2_managed(key, pattern=f"ros2 {shlex.join(args)}") + # Reconcile only a process that this runtime instance actually owns. A + # command-pattern fallback can match a robot-vendor launch with the same + # package and launch file, which would let starting a Blacknode session + # terminate the robot's normal bringup. Native children remain in the + # Runtime systemd control group and are stopped through their Popen handle + # or by systemd when Runtime exits. + stop_ros2_managed(key) backend = detect_backend()["backend"] if backend == "none": return {"ok": False, "backend": backend, "error": _NO_BACKEND_HELP} @@ -1460,25 +1466,32 @@ def wait_for_topic_interfaces( def stop_ros2_managed(key: str, pattern: str = "") -> dict[str, Any]: - """Stop one named background process.""" + """Stop one named background process owned by Blacknode. + + ``pattern`` remains accepted for saved-node and third-party compatibility, + but native process matching is intentionally not used: an identical ROS 2 + command may belong to the robot's vendor bringup. Docker matching is safe + only for the internally recorded pattern in Blacknode's dedicated helper + container. + """ backend = detect_backend()["backend"] stopped = 0 proc = _managed_detached.pop(key, None) docker_pattern = _managed_docker_patterns.pop(key, "") - pattern = pattern or docker_pattern if proc is not None and _terminate_process(proc): stopped += 1 - if backend == "native" and pattern and shutil.which("pkill"): - result = _run(["pkill", "-f", pattern], 15) - if result.returncode not in (0, 1): - return {"ok": False, "backend": backend, "stopped": stopped, "error": result.stderr.strip() or "pkill failed"} - stopped += 1 if result.returncode == 0 else 0 - if backend == "docker" and pattern: - result = _run(["docker", "exec", CONTAINER, "pkill", "-f", pattern], 15) + if backend == "docker" and docker_pattern: + result = _run(["docker", "exec", CONTAINER, "pkill", "-f", docker_pattern], 15) if result.returncode not in (0, 1): return {"ok": False, "backend": backend, "stopped": stopped, "error": result.stderr.strip() or "pkill failed"} stopped += 1 if result.returncode == 0 else 0 - return {"ok": True, "backend": backend, "stopped": stopped} + return { + "ok": True, + "backend": backend, + "stopped": stopped, + "owned_only": True, + "pattern_ignored": bool(pattern and not docker_pattern), + } def _topic_subscriber_script() -> Path: diff --git a/tests/test_ros2_nodes.py b/tests/test_ros2_nodes.py index 62358b9..206da2e 100644 --- a/tests/test_ros2_nodes.py +++ b/tests/test_ros2_nodes.py @@ -2053,10 +2053,7 @@ def fake_stop(key, pattern=""): assert result["launched"] is False assert result["run_id"] == "front_camera" - assert captured == { - "key": "front_camera", - "pattern": "ros2 launch camera_bringup", - } + assert captured == {"key": "front_camera", "pattern": ""} def test_topic_interface_inspection_reports_rgbd_publishers(monkeypatch): @@ -2343,10 +2340,28 @@ def fake_stop(key, pattern=""): "executable": "camera_node", }) assert result["running"] is False - assert captured == {"key": "camera_driver", "pattern": "ros2 run demo_camera camera_node"} + assert captured == {"key": "camera_driver", "pattern": ""} assert "stopped 1" in result["report"] +def test_native_managed_stop_never_pattern_kills_vendor_process(monkeypatch): + calls = [] + monkeypatch.setattr(rt, "detect_backend", lambda refresh=False: {"backend": "native"}) + monkeypatch.setattr(rt, "_run", lambda *args, **kwargs: calls.append((args, kwargs))) + rt._managed_detached.clear() + + result = rt.stop_ros2_managed( + "blacknode_slam", + pattern="ros2 launch slam slam.launch.py", + ) + + assert result["ok"] is True + assert result["stopped"] == 0 + assert result["owned_only"] is True + assert result["pattern_ignored"] is True + assert calls == [] + + def test_package_executables_lists_registered_commands(monkeypatch): fake = { "ok": True,