Skip to content

fix(open3d): leak#1386

Merged
paul-nechifor merged 1 commit intodevfrom
paul/fix/open3d-leak
Mar 1, 2026
Merged

fix(open3d): leak#1386
paul-nechifor merged 1 commit intodevfrom
paul/fix/open3d-leak

Conversation

@paul-nechifor
Copy link
Contributor

@paul-nechifor paul-nechifor commented Mar 1, 2026

Problem

Solution

  • Several things weren't being closed.

Breaking Changes

None.

How to Test

./bin/pytest-slow

It should end with no Open3D warnings at the end and exit code 0.

Contributor License Agreement

  • I have read and approved the CLA.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 1, 2026

Greptile Summary

This PR addresses multiple memory leaks in the Open3D integration by implementing proper cleanup mechanisms. The changes include breaking reference cycles in module shutdown, converting function-level caching to instance-level caching for bounding box calculations, and ensuring eager cleanup of tensor-tracked objects before C++ static destructors run.

Key changes:

  • Breaks In/Outowner reference cycles in ModuleBase._close_module() to enable refcount-based cleanup
  • Converts @functools.cache to @functools.cached_property in PointCloud2 for better per-instance memory management
  • Adds eager cleanup of VBG and hashmap objects in VoxelGridMapper.stop()
  • Adds cache_clear() calls to test fixtures for proper test cleanup
  • Skips test_carving due to an unresolved leak that requires further investigation

Confidence Score: 4/5

  • Safe to merge with one known unresolved issue tracked for future work
  • The PR successfully addresses multiple memory leaks through proper cleanup mechanisms and reference cycle breaking. The changes are well-documented and focused. Score reduced from 5 to 4 because test_carving was disabled rather than fixed, indicating an unresolved leak that needs investigation.
  • dimos/mapping/test_voxels.py - contains disabled test that needs future investigation

Important Files Changed

Filename Overview
dimos/core/module.py Breaks In/Out → owner reference cycles to enable refcount-based cleanup instead of waiting for GC
dimos/mapping/test_voxels.py Skips test_carving due to unresolved Open3D memory leak when process finishes
dimos/mapping/voxels.py Eagerly frees VBG and hashmap objects in stop() to prevent Open3D from reporting leaks after C++ destructors
dimos/msgs/sensor_msgs/PointCloud2.py Converts @functools.cache to @functools.cached_property for bounding box methods, enabling per-instance cleanup

Last reviewed commit: d98de58

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@paul-nechifor paul-nechifor force-pushed the paul/fix/open3d-leak branch 2 times, most recently from 5c34d11 to ee33084 Compare March 1, 2026 01:45
@paul-nechifor paul-nechifor force-pushed the paul/fix/open3d-leak branch from ee33084 to cc39cee Compare March 1, 2026 02:11
@paul-nechifor paul-nechifor merged commit 674c2eb into dev Mar 1, 2026
12 checks passed
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.

2 participants