Skip to content

Commit ca5bdaf

Browse files
Fix PhysicsManager.clear_callbacks ID-recycle bug
clear_callbacks() reset _callback_id back to 0, so subsequent register_callback() calls handed out IDs that collided with old CallbackHandle.id values still held by not-yet-garbage-collected sensors / assets. When the old object's __del__ eventually ran it deregistered the NEW callback by ID — leaving the new sensor's _initialize_callback wired up but never fired. The symptom under kitless OVPhysX was a sensor that registered its callback, then PHYSICS_READY dispatched, then sensor.reset() crashed with 'has no attribute _ALL_ENV_MASK' (init never ran). The PhysX backend hid this because its callback registry is the Carbonite event bus rather than this Python dict. Keep _callback_id monotonic across clear_callbacks invocations to preserve uniqueness for the lifetime of the process.
1 parent 38e076b commit ca5bdaf

2 files changed

Lines changed: 27 additions & 8 deletions

File tree

source/isaaclab/isaaclab/physics/physics_manager.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,23 @@ def dispatch_event(cls, event: PhysicsEvent, payload: Any = None) -> None:
156156

157157
@classmethod
158158
def clear_callbacks(cls) -> None:
159-
"""Remove all registered callbacks."""
159+
"""Remove all registered callbacks.
160+
161+
Do NOT reset ``_callback_id`` — handle IDs must remain monotonically
162+
unique across the lifetime of the process. Resetting the counter
163+
would let a future :meth:`register_callback` hand out an ID that an
164+
old, still-alive :class:`CallbackHandle` (e.g. on a sensor that has
165+
not been garbage-collected yet) holds, so when the old object
166+
eventually finalizes its ``__del__`` would deregister the new
167+
callback. This bit ovphysx's kitless multi-context tests where two
168+
``InteractiveScene``s are created in sequence: the first scene's
169+
sensor would post-GC deregister the second scene's
170+
``_initialize_callback`` by ID collision, leaving the second sensor
171+
forever uninitialized.
172+
"""
160173
for cid in list(cls._callbacks.keys()):
161174
cls.deregister_callback(cid)
162175
cls._callbacks.clear()
163-
cls._callback_id = 0
164176

165177
@classmethod
166178
def _wrap_weak_ref(cls, callback: Callable) -> Callable:

source/isaaclab_ovphysx/isaaclab_ovphysx/sensors/contact_sensor/contact_sensor.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -229,13 +229,20 @@ def _initialize_impl(self) -> None:
229229
)
230230

231231
# Optional: pose tracking via a RIGID_BODY_POSE tensor binding.
232-
# ovphysx uses fnmatch and does not brace-expand, so we widen to a single
233-
# "*" leaf pattern under the base glob. This relies on the prim_path
234-
# already isolating the sensor bodies (e.g. ".*_FOOT" matches all four
235-
# feet and no siblings). The post-bind count check below catches a
236-
# mismatch.
232+
# ovphysx fnmatch does not brace-expand, so we cannot match multiple
233+
# body names with a single glob. Single-body sensors (the common case
234+
# — one prim path per sensor) use a tight per-body pattern. Multi-body
235+
# sensors are rejected here; they need per-body bindings + an
236+
# interleaved-read kernel that does not exist yet.
237237
if self.cfg.track_pose:
238-
single_pose_pattern = f"{base_glob}/*"
238+
if self._num_sensors != 1:
239+
raise NotImplementedError(
240+
"ovphysx ContactSensor.track_pose is not yet supported for sensors that "
241+
f"resolve to more than one body per env (got {self._num_sensors} bodies "
242+
f"under '{self.cfg.prim_path}'). Workaround: create one ContactSensor "
243+
"per body."
244+
)
245+
single_pose_pattern = f"{base_glob}/{body_names[0]}"
239246
self._pose_binding = physx_instance.create_tensor_binding(
240247
pattern=single_pose_pattern,
241248
tensor_type=TT.RIGID_BODY_POSE,

0 commit comments

Comments
 (0)