Skip to content

Commit 94bef08

Browse files
committed
test: add service locator unit tests and changelog fragment
- 7 unit tests covering get_service, set_service, replacement lifecycle, close-on-clear_instance, multiple service types, and idempotent re-registration - Changelog entry for the new service locator API
1 parent 596bedb commit 94bef08

4 files changed

Lines changed: 290 additions & 35 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Added
2+
^^^^^
3+
4+
* Added :class:`~isaaclab.sim.ServiceLocator` and exposed it as
5+
:attr:`~isaaclab.sim.SimulationContext.services`.
6+
7+
Backend-specific caches can be registered and retrieved using subscript
8+
syntax (``services[cls] = instance``, ``services[cls]``). Services with
9+
a ``close()`` method are automatically closed on ``clear_instance()``.
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md).
2+
# All rights reserved.
3+
#
4+
# SPDX-License-Identifier: BSD-3-Clause
5+
6+
"""Typed service locator for lifecycle-managed singletons."""
7+
8+
from __future__ import annotations
9+
10+
from typing import TypeVar
11+
12+
_T = TypeVar("_T")
13+
14+
15+
def _try_close(service: object) -> None:
16+
"""Call close() on *service* if it exists and is callable."""
17+
close = getattr(service, "close", None)
18+
if callable(close):
19+
close()
20+
21+
22+
class ServiceLocator:
23+
"""A typed service registry keyed by class, interface, or abstract base class.
24+
25+
Services are registered and retrieved using subscript syntax::
26+
27+
locator[FabricStageCache] = FabricStageCache(stage)
28+
cache = locator[FabricStageCache]
29+
30+
Deleting a service calls ``close()`` on it if available::
31+
32+
del locator[FabricStageCache]
33+
34+
All registered services are closed and cleared via :meth:`close_all`.
35+
"""
36+
37+
def __init__(self) -> None:
38+
self._services: dict[type, object] = {}
39+
40+
def __getitem__(self, cls: type[_T]) -> _T | None:
41+
"""Retrieve a service by its key class, or ``None`` if not registered."""
42+
return self._services.get(cls) # type: ignore[return-value]
43+
44+
def __setitem__(self, cls: type[_T], instance: _T) -> None:
45+
"""Register a service under the given key.
46+
47+
The key can be the concrete class of *instance*, a parent class,
48+
or an abstract base class / protocol — allowing retrieval by
49+
interface rather than implementation.
50+
51+
Does *not* close a previously registered service — the caller is
52+
responsible for closing the old instance before replacing it.
53+
Use ``del locator[cls]`` or :meth:`pop` to close and remove.
54+
"""
55+
self._services[cls] = instance
56+
57+
def __delitem__(self, cls: type) -> None:
58+
"""Close and remove a service.
59+
60+
Calls ``close()`` on the instance if it has one, then removes it.
61+
62+
Raises:
63+
KeyError: If no service is registered under *cls*.
64+
"""
65+
instance = self._services.pop(cls)
66+
_try_close(instance)
67+
68+
def __contains__(self, cls: type) -> bool:
69+
"""Check if a service is registered under *cls*."""
70+
return cls in self._services
71+
72+
def pop(self, cls: type[_T]) -> _T | None:
73+
"""Remove and return a service without closing it.
74+
75+
Returns:
76+
The previously registered instance, or ``None`` if not registered.
77+
"""
78+
return self._services.pop(cls, None) # type: ignore[return-value]
79+
80+
def close_all(self, caught_exceptions: list[Exception] | None) -> None:
81+
"""Close all registered services and clear the registry.
82+
83+
Calls ``close()`` on each service that has one.
84+
85+
Args:
86+
caught_exceptions: If ``None``, the first exception raised by a
87+
service's ``close()`` is propagated immediately (fail-fast).
88+
If a list is provided, exceptions are appended to it and
89+
closing continues for all remaining services (collect mode).
90+
The registry is always cleared regardless of exceptions.
91+
"""
92+
services = list(self._services.values())
93+
self._services.clear()
94+
for service in services:
95+
try:
96+
_try_close(service)
97+
except Exception as e:
98+
if caught_exceptions is None:
99+
raise
100+
caught_exceptions.append(e)

source/isaaclab/isaaclab/sim/simulation_context.py

Lines changed: 17 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
import traceback
1212
from collections.abc import Iterator
1313
from contextlib import contextmanager
14-
from typing import TYPE_CHECKING, Any, TypeVar
14+
from typing import TYPE_CHECKING, Any
1515

1616
import toml
1717
import torch
@@ -30,6 +30,7 @@
3030
)
3131
from isaaclab.renderers.render_context import RenderContext
3232
from isaaclab.scene.scene_data_provider import SceneDataProvider
33+
from isaaclab.sim.service_locator import ServiceLocator
3334
from isaaclab.sim.utils import create_new_stage
3435
from isaaclab.utils.string import clear_resolve_matching_names_cache
3536
from isaaclab.utils.version import has_kit
@@ -38,8 +39,6 @@
3839
if TYPE_CHECKING:
3940
from isaaclab.cloner.clone_plan import ClonePlan
4041

41-
_T = TypeVar("_T")
42-
4342
from .simulation_cfg import SimulationCfg
4443
from .spawners import DomeLightCfg, GroundPlaneCfg
4544

@@ -216,10 +215,7 @@ def __init__(self, cfg: SimulationCfg | None = None):
216215
order=5,
217216
)
218217

219-
# Singleton service registry — backend-specific caches register themselves
220-
# here, keyed by their class. Services with a ``close()`` method are closed
221-
# when the SimulationContext is torn down via ``clear_instance()``.
222-
self._services: dict[type, object] = {}
218+
self._services = ServiceLocator()
223219

224220
type(self)._instance = self # Mark as valid singleton only after successful init
225221

@@ -863,33 +859,17 @@ def get_setting(self, name: str) -> Any:
863859
# Service locator
864860
# ------------------------------------------------------------------
865861

866-
def get_service(self, cls: type[_T]) -> _T | None:
867-
"""Retrieve a registered singleton service by its class.
868-
869-
Args:
870-
cls: The service class used as key.
871-
872-
Returns:
873-
The registered instance, or ``None`` if not registered.
874-
"""
875-
return self._services.get(cls) # type: ignore[return-value]
876-
877-
def set_service(self, cls: type[_T], instance: _T) -> None:
878-
"""Register a singleton service, keyed by its class.
862+
@property
863+
def services(self) -> ServiceLocator:
864+
"""Typed service registry for backend-specific singletons.
879865
880-
Overwrites any previously registered instance for the same class.
881-
If the old instance has a ``close()`` method it is called before
882-
replacement. Services are automatically closed and cleared when
883-
:meth:`clear_instance` is called.
866+
Usage::
884867
885-
Args:
886-
cls: The service class used as key.
887-
instance: The service instance to register.
868+
sim_context.services[FabricStageCache] = cache
869+
cache = sim_context.services[FabricStageCache]
870+
del sim_context.services[FabricStageCache] # closes and removes
888871
"""
889-
old = self._services.get(cls)
890-
if old is not None and old is not instance and hasattr(old, "close"):
891-
old.close()
892-
self._services[cls] = instance
872+
return self._services
893873

894874
@classmethod
895875
def clear_instance(cls) -> None:
@@ -905,10 +885,8 @@ def clear_instance(cls) -> None:
905885
cls._instance._visualizers.clear()
906886

907887
# Close and drop all registered singleton services
908-
for service in cls._instance._services.values():
909-
if hasattr(service, "close"):
910-
service.close()
911-
cls._instance._services.clear()
888+
service_errors: list[Exception] = []
889+
cls._instance._services.close_all(caught_exceptions=service_errors)
912890

913891
# Tear down the stage. We skip clear_stage() (prim-by-prim deletion) since
914892
# close_stage() + app shutdown destroy the entire stage at once.
@@ -920,6 +898,10 @@ def clear_instance(cls) -> None:
920898
# Clear instance
921899
cls._instance = None
922900

901+
if service_errors:
902+
msg = f"SimulationContext.clear_instance(): {len(service_errors)} service(s) failed to close"
903+
raise RuntimeError(msg) from service_errors[0]
904+
923905
gc.collect()
924906
logger.info("SimulationContext cleared")
925907

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md).
2+
# All rights reserved.
3+
#
4+
# SPDX-License-Identifier: BSD-3-Clause
5+
6+
"""Tests for ServiceLocator."""
7+
8+
import pytest
9+
10+
from isaaclab.sim.service_locator import ServiceLocator
11+
12+
13+
class _DummyServiceWithClose:
14+
"""Service with a callable close() method."""
15+
16+
def __init__(self):
17+
self.closed = False
18+
19+
def close(self):
20+
self.closed = True
21+
22+
23+
class _DummyServiceWithoutClose:
24+
"""Service without a close() method."""
25+
26+
pass
27+
28+
29+
class _DummyServiceWithCloseProperty:
30+
"""Service where 'close' exists but is not callable."""
31+
32+
close = 42 # attribute, not a method
33+
34+
35+
class _DummyServiceThatThrows:
36+
"""Service whose close() raises an exception."""
37+
38+
def close(self):
39+
raise RuntimeError("close failed")
40+
41+
42+
class TestServiceLocator:
43+
"""Test ServiceLocator subscript API."""
44+
45+
def test_get_returns_none_when_unregistered(self):
46+
loc = ServiceLocator()
47+
assert loc[_DummyServiceWithClose] is None
48+
49+
def test_set_and_get(self):
50+
loc = ServiceLocator()
51+
svc = _DummyServiceWithClose()
52+
loc[_DummyServiceWithClose] = svc
53+
assert loc[_DummyServiceWithClose] is svc
54+
55+
def test_contains(self):
56+
loc = ServiceLocator()
57+
assert _DummyServiceWithClose not in loc
58+
loc[_DummyServiceWithClose] = _DummyServiceWithClose()
59+
assert _DummyServiceWithClose in loc
60+
61+
def test_del_closes_service(self):
62+
loc = ServiceLocator()
63+
svc = _DummyServiceWithClose()
64+
loc[_DummyServiceWithClose] = svc
65+
del loc[_DummyServiceWithClose]
66+
assert svc.closed
67+
assert loc[_DummyServiceWithClose] is None
68+
69+
def test_del_without_close_method(self):
70+
"""del on a service without close() should not raise."""
71+
loc = ServiceLocator()
72+
loc[_DummyServiceWithoutClose] = _DummyServiceWithoutClose()
73+
del loc[_DummyServiceWithoutClose]
74+
assert loc[_DummyServiceWithoutClose] is None
75+
76+
def test_del_with_non_callable_close_property(self):
77+
"""del on a service where close is a property (not callable) should not raise."""
78+
loc = ServiceLocator()
79+
svc = _DummyServiceWithCloseProperty()
80+
loc[_DummyServiceWithCloseProperty] = svc
81+
del loc[_DummyServiceWithCloseProperty]
82+
assert loc[_DummyServiceWithCloseProperty] is None
83+
84+
def test_del_missing_raises_key_error(self):
85+
loc = ServiceLocator()
86+
with pytest.raises(KeyError):
87+
del loc[_DummyServiceWithClose]
88+
89+
def test_pop_returns_without_closing(self):
90+
loc = ServiceLocator()
91+
svc = _DummyServiceWithClose()
92+
loc[_DummyServiceWithClose] = svc
93+
popped = loc.pop(_DummyServiceWithClose)
94+
assert popped is svc
95+
assert not svc.closed
96+
assert loc[_DummyServiceWithClose] is None
97+
98+
def test_pop_missing_returns_none(self):
99+
loc = ServiceLocator()
100+
assert loc.pop(_DummyServiceWithClose) is None
101+
102+
def test_close_all(self):
103+
loc = ServiceLocator()
104+
svc1 = _DummyServiceWithClose()
105+
svc2 = _DummyServiceWithoutClose()
106+
loc[_DummyServiceWithClose] = svc1
107+
loc[_DummyServiceWithoutClose] = svc2
108+
loc.close_all(caught_exceptions=None)
109+
assert svc1.closed
110+
assert loc[_DummyServiceWithClose] is None
111+
assert loc[_DummyServiceWithoutClose] is None
112+
113+
def test_close_all_skips_non_callable_close(self):
114+
"""close_all does not crash on services with non-callable close attribute."""
115+
loc = ServiceLocator()
116+
loc[_DummyServiceWithCloseProperty] = _DummyServiceWithCloseProperty()
117+
loc.close_all(caught_exceptions=None) # should not raise
118+
assert loc[_DummyServiceWithCloseProperty] is None
119+
120+
def test_close_all_fail_fast_raises(self):
121+
"""With caught_exceptions=None, first failure propagates immediately."""
122+
loc = ServiceLocator()
123+
loc[_DummyServiceThatThrows] = _DummyServiceThatThrows()
124+
with pytest.raises(RuntimeError, match="close failed"):
125+
loc.close_all(caught_exceptions=None)
126+
# Registry is cleared even on failure
127+
assert loc[_DummyServiceThatThrows] is None
128+
129+
def test_close_all_collect_mode(self):
130+
"""With a list, exceptions are collected and all services are closed."""
131+
loc = ServiceLocator()
132+
svc_ok = _DummyServiceWithClose()
133+
loc[_DummyServiceWithClose] = svc_ok
134+
loc[_DummyServiceThatThrows] = _DummyServiceThatThrows()
135+
errors: list[Exception] = []
136+
loc.close_all(caught_exceptions=errors)
137+
assert svc_ok.closed
138+
assert len(errors) == 1
139+
assert isinstance(errors[0], RuntimeError)
140+
assert loc[_DummyServiceWithClose] is None
141+
assert loc[_DummyServiceThatThrows] is None
142+
143+
def test_multiple_service_types(self):
144+
loc = ServiceLocator()
145+
svc1 = _DummyServiceWithClose()
146+
svc2 = _DummyServiceWithoutClose()
147+
loc[_DummyServiceWithClose] = svc1
148+
loc[_DummyServiceWithoutClose] = svc2
149+
assert loc[_DummyServiceWithClose] is svc1
150+
assert loc[_DummyServiceWithoutClose] is svc2
151+
152+
def test_base_class_key(self):
153+
"""Can register under a base class and retrieve by it."""
154+
155+
class Base:
156+
pass
157+
158+
class Impl(Base):
159+
pass
160+
161+
loc = ServiceLocator()
162+
impl = Impl()
163+
loc[Base] = impl
164+
assert loc[Base] is impl

0 commit comments

Comments
 (0)