Fix memory leak in ManagerBasedEnv.close

[cvolkcvolk]

Description

ManagerBasedEnv.close() leaves two kinds of per-instance state alive after it returns. Gymnasium's wrapper chain (OrderEnforcing plus the make() registry) keeps the wrapped env reachable, so anything the env still references at close stays in memory:

1. self.obs_buf: the dict of cached observation tensors from the last step() / reset(). For image sensors this is tens of MB per camera.
2. self.observation_space / self.action_space (plus the single_*_space variants on ManagerBasedRLEnv). Each image-observation gym.spaces.Box pins ~110 MB of bounds arrays (low, high, bounded_below, bounded_above) at shape (4, 720, 1280, 3).

So roughly one env's worth of bounds arrays and the last observation buffer survive for the process lifetime. Evaluation loops that rebuild envs across tasks accumulate these and eventually OOM.

The fix clears obs_buf and nulls the space attributes in close().

Found while debugging an evaluation-sweep OOM in a downstream project. No associated issue.

Fixes # (no associated issue)

Testing

Regression test at source/isaaclab/test/envs/test_manager_based_env_close_leak.py builds Isaac-Cartpole-v0 via gym.make, runs reset and one step, calls close(), then asserts obs_buf is empty and the space attributes are None. Parametrised over cuda:0 and cpu. It fails on the pre-fix close() and passes with the fix.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with ./isaaclab.sh --format
• I have made corresponding changes to the documentation (N/A, internal lifecycle method)
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have updated the changelog and the corresponding version in the extension's config/extension.toml file (happy to add a fragment if maintainers want a patch-bump entry)
• I have added my name to the CONTRIBUTORS.md (happy to add if maintainers want)

[greptile-apps]

Greptile Summary

This PR fixes a memory leak in ManagerBasedEnv.close() and ManagerBasedRLEnv.close() where gymnasium's wrapper chain kept the closed env reachable, preventing GC of observation tensor buffers and gym.spaces.Box bounds arrays. A regression test is included.

• manager_based_env.py: Calls self.obs_buf.clear() at the start of close(), releasing references to cached observation tensors (potentially tens of MB per image sensor) before any manager is torn down.
• manager_based_rl_env.py: Nulls all four gym space attributes (single_observation_space, single_action_space, observation_space, action_space) after deleting RL managers and before delegating to super().close(), releasing gym.spaces.Box bounds arrays that can be ~110 MB each for image-observation envs.
• Test: New parametrized test (cuda:0 / cpu) builds Isaac-Cartpole-v0, runs reset + step + close, and asserts obs_buf is empty and observation_space / action_space are None; does not assert the single_*_space variants.

Confidence Score: 4/5

The fix is narrowly scoped to the teardown path and does not alter any live-env behavior; safe to merge.

Both fixes are correct and well-placed. The only gap is that the new test doesn't assert single_observation_space and single_action_space are None after close, leaving part of the fix unverified by the regression test.

The test file would benefit from adding assertions for single_observation_space and single_action_space to fully cover the four attributes nulled in ManagerBasedRLEnv.close().

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/envs/manager_based_env.py	Adds obs_buf.clear() before manager teardown in close(), releasing cached observation tensor references before gymnasium's wrapper chain can pin them.
source/isaaclab/isaaclab/envs/manager_based_rl_env.py	Nulls all four gym space attributes (single_observation_space, single_action_space, observation_space, action_space) in close() before delegating to the parent, releasing gym.spaces.Box bounds-array memory.
source/isaaclab/test/envs/test_manager_based_env_close_leak.py	New regression test for the close-leak fix; exercises reset + step + close and asserts obs_buf and spaces are cleared, but does not assert single_observation_space/single_action_space are None.

Sequence Diagram

sequenceDiagram
    participant W as Gymnasium Wrapper Chain
    participant RL as ManagerBasedRLEnv
    participant B as ManagerBasedEnv

    W->>RL: close()
    RL->>RL: del command_manager, reward_manager, termination_manager, curriculum_manager
    RL->>RL: "single_observation_space = None"
    RL->>RL: "single_action_space = None"
    RL->>RL: "observation_space = None"
    RL->>RL: "action_space = None"
    Note over RL: Box bounds arrays (~110 MB ea.) released
    RL->>B: super().close()
    B->>B: sim.stop()
    B->>B: obs_buf.clear()
    Note over B: Tensor references dropped
    B->>B: del viewport_camera_controller, action_manager, observation_manager, event_manager, recorder_manager, scene
    B->>B: sim.clear_instance()
    B->>B: "_is_closed = True"
    Note over W: Wrapper chain still holds env ref, but large data is now freed

Loading

_{Reviews (1): Last reviewed commit: "Add regression test for ManagerBasedEnv...." | Re-trigger Greptile}

[greptile-apps]

Missing assertions for single_observation_space / single_action_space

The fix in ManagerBasedRLEnv.close() nulls all four space attributes (single_observation_space, single_action_space, observation_space, action_space), but the test only asserts the last two. The single_* variants are the per-environment gym.spaces.Box objects described in the PR description as holding ~110 MB of bounds arrays for image observations — they are exactly the attributes whose memory the fix aims to release. Leaving them unchecked means a regression that stops nulling single_observation_space would go undetected.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[isaaclab-review-bot]

Code Review: Fix memory leak in ManagerBasedEnv.close

Good fix for a real and well-diagnosed issue. The PR description is thorough, the repro methodology is solid, and the fix is targeted. A few observations and suggestions below.

---

Findings

1. [Medium] self.extras dict not cleared in close()

File: source/isaaclab/isaaclab/envs/manager_based_env.py, line 557 area

self.extras is allocated at line 155 and accumulates log dicts with metrics each step. While the individual values are lightweight scalars/dicts (not large tensors), for consistency with the obs_buf cleanup it would be good to clear it:

self.extras.clear()

This is a minor nit — the memory impact is negligible compared to obs_buf and spaces — but it makes the "release everything on close" contract more explicit.

---

2. [Low] Defensive guard style inconsistency

File: source/isaaclab/isaaclab/envs/manager_based_env.py, line 556

if isinstance(getattr(self, "obs_buf", None), dict):
    self.obs_buf.clear()

The defensive getattr + isinstance guard is wise for obs_buf (since close() could theoretically be called before __init__ fully completes), but the space nulling in manager_based_rl_env.py uses direct attribute assignment without a similar guard:

self.single_observation_space = None  # could AttributeError if __init__ never ran _configure_gym_env_spaces

If close() is ever called on a partially-constructed env (e.g., if scene creation throws), these would raise AttributeError. Consider wrapping in a similar pattern:

for attr in ("single_observation_space", "single_action_space", "observation_space", "action_space"):
    if hasattr(self, attr):
        setattr(self, attr, None)

---

3. [Medium] Same leak exists in DirectRLEnv.close()

File: source/isaaclab/isaaclab/envs/direct_rl_env.py, line 524

DirectRLEnv.close() (line 524) has the same pattern: it doesn't release self.obs_buf, self.observation_space, self.action_space, self.single_observation_space, or self.single_action_space. Since DirectRLEnv also inherits from gym.Env and uses gym.make(), it's subject to the same wrapper-chain retention.

This is out of scope for this PR but worth noting — a follow-up would complete the fix across the env hierarchy.

---

4. [Low] Test assumes cuda:0 availability

File: source/isaaclab/test/envs/test_manager_based_env_close_leak.py, line 24

@pytest.mark.parametrize("device", ["cuda:0", "cpu"])

The cuda:0 parametrization will fail on CPU-only CI runners. Consider gating with:

import pytest

devices = ["cpu"]
if torch.cuda.is_available():
    devices.append("cuda:0")

@pytest.mark.parametrize("device", devices)

Or using pytest.param("cuda:0", marks=pytest.mark.skipif(not torch.cuda.is_available(), reason="CUDA not available")).

(This may already be handled by the test infrastructure's runner selection — but it's worth confirming.)

---

5. [Low] Test doesn't verify single_observation_space / single_action_space

File: source/isaaclab/test/envs/test_manager_based_env_close_leak.py, lines 46-48

The test asserts observation_space and action_space are None after close, but doesn't check the single_* variants that are also nulled in the fix. For completeness:

assert env.unwrapped.single_observation_space is None, "single_observation_space was not released on close"
assert env.unwrapped.single_action_space is None, "single_action_space was not released on close"

---

Summary

Severity	Count
Medium	2
Low	3

The fix is correct, well-scoped, and addresses a genuine memory leak that compounds in evaluation loops. The implementation is clean. The two medium items (clearing extras and the parallel leak in DirectRLEnv) are nice-to-haves rather than blockers. Overall this is a solid contribution.