Skip to content

[Bug] Fix ndarray memory leak#278

Merged
hughperkins merged 15 commits intomainfrom
hp/ndarray-mem-leak
Nov 20, 2025
Merged

[Bug] Fix ndarray memory leak#278
hughperkins merged 15 commits intomainfrom
hp/ndarray-mem-leak

Conversation

@hughperkins
Copy link
Copy Markdown
Collaborator

@hughperkins hughperkins commented Nov 17, 2025

Issue: #

Brief Summary

See taichi-dev/taichi#8764 by @BernardoCovas for more details.

copilot:summary

Walkthrough

copilot:walkthrough

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a memory leak in ndarray objects by moving the __del__ method from the ScalarNdarray class to the base Ndarray class, ensuring that all ndarray types (scalar, vector, and matrix) properly clean up their underlying C++ objects when garbage collected.

Key Changes

  • Moved __del__ method from ScalarNdarray to Ndarray base class to ensure all ndarray subclasses benefit from proper cleanup
  • Added test case to verify that ndarray deletion properly cleans up the underlying C++ object

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
python/gstaichi/lang/_ndarray.py Moved __del__ from ScalarNdarray (removed lines 279-284) to Ndarray base class (added lines 41-45), ensuring VectorNdarray and MatrixNdarray also have proper cleanup
tests/python/test_ndarray.py Added test_ndarray_del to verify that the C++ ndarray object is properly deleted when the Python wrapper goes out of scope

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/python/test_ndarray.py Outdated
Comment thread tests/python/test_ndarray.py Outdated
Comment thread tests/python/test_ndarray.py Outdated
Comment thread python/gstaichi/lang/_ndarray.py Outdated
Comment on lines +42 to +46
if impl is not None and impl.get_runtime is not None and impl.get_runtime() is not None:
prog = impl.get_runtime()._prog
if prog is not None:
prog.delete_ndarray(self.arr)

Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

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

Overly complex 'del' method.

Suggested change
if impl is not None and impl.get_runtime is not None and impl.get_runtime() is not None:
prog = impl.get_runtime()._prog
if prog is not None:
prog.delete_ndarray(self.arr)
pass
def release(self):
if impl is not None and impl.get_runtime is not None and impl.get_runtime() is not None:
prog = impl.get_runtime()._prog
if prog is not None:
prog.delete_ndarray(self.arr)

Copilot uses AI. Check for mistakes.
Comment thread python/gstaichi/lang/_ndarray.py Outdated
Comment thread gstaichi/python/export_lang.cpp Outdated
@hughperkins hughperkins enabled auto-merge (squash) November 20, 2025 15:09
@hughperkins hughperkins merged commit 981ef70 into main Nov 20, 2025
59 checks passed
@hughperkins hughperkins deleted the hp/ndarray-mem-leak branch November 20, 2025 15:53
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.

4 participants