Fix memory leak in optional_import traceback handling#8782
Fix memory leak in optional_import traceback handling#8782aymuos15 wants to merge 1 commit intoProject-MONAI:devfrom
Conversation
…I#7480, Project-MONAI#7727) When an import fails, `optional_import` captured the live traceback object and stored it in a `_LazyRaise` closure. The traceback held references to stack frames containing CUDA tensors, preventing garbage collection and causing GPU memory leaks. Replace the live traceback with a string-formatted copy via `traceback.format_exception()` and clear the original with `import_exception.__traceback__ = None`. The formatted traceback is appended to the error message so debugging information is preserved. Also move three hot-path `optional_import` calls in `monai/transforms/utils.py` (cucim.skimage, cucim morphology EDT) from function bodies to module level, eliminating repeated leaked tracebacks on every invocation. Fixes Project-MONAI#7480, fixes Project-MONAI#7727 Signed-off-by: Soumya Snigdha Kundu <soumya_snigdha.kundu@kcl.ac.uk>
📝 WalkthroughWalkthroughThe changes refactor optional CucIM/CuPy dependency handling to prevent GPU memory leaks. Private import flags and guards are introduced in Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/utils/test_optional_import.py (1)
80-97: Prefix unused variable with underscore.Line 91:
modis never used. Rename to_modto indicate intentional discard.🔧 Suggested fix
- mod, flag = optional_import("nonexistent_module_for_leak_test") + _mod, flag = optional_import("nonexistent_module_for_leak_test")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/utils/test_optional_import.py` around lines 80 - 97, In test_no_traceback_leak, the unused variable `mod` returned from optional_import should be renamed to `_mod` to signal intentional discard; update the tuple unpacking in the _do_import function from `mod, flag = optional_import("nonexistent_module_for_leak_test")` to `_mod, flag = optional_import("nonexistent_module_for_leak_test")` (keeping the rest of test_no_traceback_leak, _Marker, and gc check unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/utils/test_optional_import.py`:
- Around line 80-97: In test_no_traceback_leak, the unused variable `mod`
returned from optional_import should be renamed to `_mod` to signal intentional
discard; update the tuple unpacking in the _do_import function from `mod, flag =
optional_import("nonexistent_module_for_leak_test")` to `_mod, flag =
optional_import("nonexistent_module_for_leak_test")` (keeping the rest of
test_no_traceback_leak, _Marker, and gc check unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7b9386de-fac6-49d8-a44b-50a0d17f850d
📒 Files selected for processing (3)
monai/transforms/utils.pymonai/utils/module.pytests/utils/test_optional_import.py
Summary
optional_importwhere stored traceback objects retain references to entire call stacks, preventing garbage collectionimport_exception.__traceback__ = Noneoptional_importcalls for cucim to module level inmonai/transforms/utils.pyFixes #7480, #7727
Details
monai/utils/module.py:__traceback__object withtraceback.format_exception()stringimport_exception.__traceback__ = Noneto break reference chainmonai/transforms/utils.py:optional_importcalls to module level (consistent with existing skimage/scipy/cupy pattern)distance_transform_edtTest plan
test_no_traceback_leak— weakref regression test for the leaktest_failed_import_shows_traceback_string— verifies traceback in error messagetest_optional_import.pytests pass