Skip to content

Release a RoboDisco env's PyBullet client on close() - #204

Open
yichao-liang wants to merge 2 commits into
masterfrom
robodisco-close-releases-client
Open

yichao-liang wants to merge 2 commits into
masterfrom
robodisco-close-releases-client

Conversation

@yichao-liang

Copy link
Copy Markdown
Collaborator

Summary

RoboDiscoEnv.close() was pass, so every env built with robodisco.make(...) kept its PyBullet client, a whole world, connected until the process exited.
scripts/robodisco_getting_started.py builds all 22 environments in one process and closes each one, and none was released.

close() disposes the wrapped env.
close() now calls the wrapped env's PyBulletEnv.dispose(), which releases the world-gap record, forgets the client's asset records and disconnects the client.

dispose() releases only once.
PyBullet gives a disconnected client's id to the next client that connects.
A repeated dispose() therefore released the newer client's records and disconnected its world.
Gymnasium allows repeated close() calls, and its env checker forwards each one to the env, turning an error into a warning.
PyBulletEnv.dispose() now does nothing after its first call, which also covers the other callers of dispose().

Cover's second client (separate commit).
PyBulletCoverEnv opens a robot-only client for the forward kinematics in step(), and dispose() left it connected.
It now disconnects that client too.

Measured on a compute node with a script that builds, resets and closes all 22 environments in one process, as the smoke script does:
on master, 28 clients stayed connected and the process grew from 885 MB to 3.3 GB;
on this branch, 5 stay connected and it grows from 868 MB to 1.3 GB.
The 5 are the module-level probe envs of Balloons, Crane, IceRink, Launcher and Magnets.
Each is one shared instance per class per process, reused by every env of its class, so they do not grow with the number of envs built.

With the earlier envs closed, the smoke script now resets all 22 environments cleanly.
Circuit, Laser and Switch pass there only because they now get client 0 for themselves: they still look up their joints in client 0, so they still fail while another env is open in the same process.
The envs README's advice to build them in a fresh process stands, and naming their client in those calls is a separate change.

Test plan

  • Reproduced on master: after env.close(), p.getConnectionInfo(physicsClientId=0) still reports isConnected: 1 for the Blocks env.
  • New test_close_disconnects_the_pybullet_client (Blocks) and test_pybullet_cover_dispose_disconnects_both_clients fail on master and pass here.
  • With the dispose() guard removed, the repeated close() disconnects the client that took the freed id, and the new test fails.
  • tests/envs/test_robodisco.py and tests/envs/test_pybullet_cover.py pass.
  • scripts/robodisco_getting_started.py passes, with 22/22 environments resetting cleanly.
  • The envs README Quick Start runs verbatim, and its env's client is disconnected after env.close().
  • CI replay of b5bb09d on Engaging: the 8 pytest shards in CI's Ubuntu 24.04 container (2528 passed, none failed) and the static checks (yapf, isort 5.10.1, docformatter 1.4, mypy, pylint) pass.

🤖 Generated with Claude Code

RoboDiscoEnv.close() did nothing, so every env built with
robodisco.make() kept its PyBullet client, a whole world, connected
until the process exited. scripts/robodisco_getting_started.py builds
all 22 envs in one process and closes each one; 28 clients stayed
connected and the process grew from 885 MB to 3.3 GB.

close() now calls the wrapped env's dispose(), which releases the
world-gap record and the asset records and disconnects the client.
Only the first dispose() does so. PyBullet gives a disconnected
client's id to the next client that connects, so a repeated call
released that client's records and disconnected its world. Gymnasium
allows repeated close() calls, and its env checker forwards each one.

The regression test closes a Blocks env and checks that its client is
disconnected, then connects until a new client takes the freed id,
closes twice more and checks that the new client is still connected.
PyBulletCoverEnv opens a second, robot-only PyBullet client for the
forward kinematics in step(), and dispose() left it connected, so
closing a robodisco/Cover-v0 env still kept a world. dispose() now
disconnects it along with the world's client, and only on the first
call.
@yichao-liang
yichao-liang enabled auto-merge (squash) October 3, 2026 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant