diff --git a/src/core/oxr/cpp/inc/oxr/oxr_session.hpp b/src/core/oxr/cpp/inc/oxr/oxr_session.hpp index 83edc15fa..617bb328e 100644 --- a/src/core/oxr/cpp/inc/oxr/oxr_session.hpp +++ b/src/core/oxr/cpp/inc/oxr/oxr_session.hpp @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 #pragma once diff --git a/src/core/oxr/cpp/oxr_session.cpp b/src/core/oxr/cpp/oxr_session.cpp index 6c62e43b2..2d57c815b 100644 --- a/src/core/oxr/cpp/oxr_session.cpp +++ b/src/core/oxr/cpp/oxr_session.cpp @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 #include "inc/oxr/oxr_session.hpp" diff --git a/src/core/oxr/python/oxr_bindings.cpp b/src/core/oxr/python/oxr_bindings.cpp index 796beea55..17612e33b 100644 --- a/src/core/oxr/python/oxr_bindings.cpp +++ b/src/core/oxr/python/oxr_bindings.cpp @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 #include @@ -6,8 +6,67 @@ #include #include +#include +#include +#include +#include + namespace py = pybind11; +namespace core +{ + +/** + * @brief Python-facing session wrapper: destroys the underlying OpenXRSession in __exit__. + * + * Binding OpenXRSession directly with a no-op __exit__ leaves destruction to the pybind + * holder, which can run at garbage collection -- after the CloudXR runtime/IPC socket has + * been torn down -- producing "Broken pipe"/invalid-handle errors. Holding the session in a + * unique_ptr and resetting it in close()/exit() tears it down deterministically at + * context-manager exit, while the runtime is still alive. Mirrors PyDeviceIOSession. + */ +class PyOpenXRSession +{ +public: + PyOpenXRSession(const std::string& app_name, const std::vector& extensions) + : impl_(std::make_unique(app_name, extensions)) + { + } + + OpenXRSessionHandles get_handles() const + { + if (!impl_) + { + throw std::runtime_error("OpenXRSession has been closed/destroyed"); + } + return impl_->get_handles(); + } + + void close() + { + impl_.reset(); + } + + PyOpenXRSession& enter() + { + if (!impl_) + { + throw std::runtime_error("OpenXRSession has been closed/destroyed"); + } + return *this; + } + + void exit(py::object, py::object, py::object) + { + close(); + } + +private: + std::unique_ptr impl_; +}; + +} // namespace core + PYBIND11_MODULE(_oxr, m) { m.doc() = "Isaac Teleop OXR - OpenXR Session Module"; @@ -40,15 +99,13 @@ PYBIND11_MODULE(_oxr, m) [](const core::OpenXRSessionHandles& self) { return reinterpret_cast(self.xrGetInstanceProcAddr); }, "Get xrGetInstanceProcAddr function pointer as integer"); - // OpenXRSession class (for creating sessions) - py::class_>(m, "OpenXRSession") + // OpenXRSession (Python-facing wrapper; see core::PyOpenXRSession for the lifetime contract) + py::class_>(m, "OpenXRSession") .def(py::init&>(), py::arg("app_name"), py::arg("extensions") = std::vector()) - .def("get_handles", &core::OpenXRSession::get_handles, "Get session handles for sharing") - .def("__enter__", [](core::OpenXRSession& self) -> core::OpenXRSession& { return self; }) - .def("__exit__", - [](core::OpenXRSession& self, py::object, py::object, py::object) - { - // RAII cleanup handled automatically when object is destroyed - }); + .def("get_handles", &core::PyOpenXRSession::get_handles, "Get session handles for sharing") + .def("close", &core::PyOpenXRSession::close, + "Release the native session immediately (usually automatic via context manager)") + .def("__enter__", &core::PyOpenXRSession::enter) + .def("__exit__", &core::PyOpenXRSession::exit); } diff --git a/src/core/teleop_session_manager/python/teleop_session.py b/src/core/teleop_session_manager/python/teleop_session.py index f7fecf94d..29bf2cc1f 100644 --- a/src/core/teleop_session_manager/python/teleop_session.py +++ b/src/core/teleop_session_manager/python/teleop_session.py @@ -214,7 +214,7 @@ def __init__(self, config: TeleopSessionConfig): @property def oxr_session(self) -> Optional[oxr.OpenXRSession]: - """The internal OpenXR session, or ``None`` when using external handles (read-only).""" + """The internal OpenXR session, or ``None`` when using external handles or after the context manager exits (read-only).""" return self._oxr_session @property @@ -1054,8 +1054,15 @@ def __exit__(self, exc_type, exc_val, exc_tb): # ExitStack automatically cleans up all managed contexts in reverse order. # Preserve TeleopSession's historical behavior of not suppressing # exceptions from the user body, even if a child context manager would. - self._exit_stack.__exit__(exc_type, exc_val, exc_tb) - self._exit_stack = ExitStack() + try: + self._exit_stack.__exit__(exc_type, exc_val, exc_tb) + self._exit_stack = ExitStack() + finally: + # The ExitStack above closes the OpenXR session; drop our reference so the + # public `oxr_session` property honors its documented None contract post-exit + # rather than surfacing a torn-down session -- even if a managed context's + # cleanup raised. (deviceio_session has no such property/contract.) + self._oxr_session = None if runner_error is not None: raise runner_error diff --git a/src/core/teleop_session_manager_tests/python/test_teleop_session.py b/src/core/teleop_session_manager_tests/python/test_teleop_session.py index 8a1f65756..43096ae6a 100644 --- a/src/core/teleop_session_manager_tests/python/test_teleop_session.py +++ b/src/core/teleop_session_manager_tests/python/test_teleop_session.py @@ -1082,6 +1082,39 @@ def test_exit_cleans_up(self): # but the exit stack has been unwound assert session._setup_complete is True + def test_exit_clears_oxr_session(self): + """After the with-block exits, the public oxr_session property honors its None contract.""" + pipeline = MockPipeline(leaf_nodes=[]) + + config = make_config(pipeline) + with mock_session_dependencies(): + session = TeleopSession(config) + with session as s: + assert s.oxr_session is not None + + assert session.oxr_session is None + + def test_exit_clears_oxr_session_even_if_cleanup_raises(self): + """oxr_session returns None post-exit even when a managed context's cleanup raises.""" + + class RaisingDeviceIOSession(MockDeviceIOSession): + def __exit__(self, *args): + raise RuntimeError("cleanup boom") + + pipeline = MockPipeline(leaf_nodes=[]) + config = make_config(pipeline) + with mock_session_dependencies(mock_dio=RaisingDeviceIOSession()): + session = TeleopSession(config) + session.__enter__() + assert session.oxr_session is not None + + # The ExitStack unwind raises, but exception propagation is preserved + # and the reference is still cleared via the finally block. + with pytest.raises(RuntimeError, match="cleanup boom"): + session.__exit__(None, None, None) + + assert session.oxr_session is None + def test_context_manager_protocol(self): """TeleopSession works correctly as a context manager.""" head = MockHeadSource()