Skip to content

BUG: fix refcount leak on overlapping copyto with where=False - #31968

Merged
mattip merged 3 commits into
numpy:mainfrom
Ethan0456:fix/copyto-where-false-overlap-refcount-leak
Jul 13, 2026
Merged

mattip merged 3 commits into
numpy:mainfrom
Ethan0456:fix/copyto-where-false-overlap-refcount-leak

Conversation

@Ethan0456

Copy link
Copy Markdown
Contributor

PR summary

This PR fixes a reference leak in PyArray_AssignArray (used by np.copyto and related assign paths) when:

  1. src/dst overlap, so an intermediate temp is allocated (copied_src = 1), and
  2. a scalar boolean where=False triggers an early return that used to skip Py_DECREF of that temp.

Success and failure paths already released the temp; the early-return path did not. With object dtypes, filling the temp INCREFs every element, so those references (and the temp array) were leaked.

Fix:
Py_DECREF(src) on the where=False early return when copied_src is set. No change to assignment semantics (where=False still writes nothing).

Test:
Regression test in test_api.py that checks object refcounts after overlapping np.copyto(..., where=False).

No release note (internal leak only; no user-visible API/behavior change).

First time committer introduction

I've been exploring the NumPy codebase recently, finding and fixing bugs. I had one PR merged last week and am continuing with small correctness fixes like this.

AI Disclosure

AI was used to help locate and draft the fix. The bug, fix, and tests were reviewed and validated by a human, and this PR was prepared and submitted by human.

PyArray_AssignArray allocated an overlap temporary then early-returned
on scalar where=False without Py_DECREF of that temp. Free the temp on
that path and add a regression test.
@charris charris added 00 - Bug 09 - Backport-Candidate PRs tagged should be backported labels Jul 11, 2026
Comment thread numpy/_core/tests/test_api.py Resolved
Drop the direct getrefcount assertion in favor of a path-exercising
regression test that leak sanitizer builds can catch, matching the
style used in similar recent leak fixes.
The free on the early-return path is self-explanatory; restore the
prior comment wording per review.

@kumaraditya303 kumaraditya303 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks.

@Ethan0456

Copy link
Copy Markdown
Contributor Author

Hi @kumaraditya303, thanks for reviewing and approving this PR! Since the requested changes have been addressed and the CI is passing, I wanted to check if there's anything else needed before it can be merged. If not, I'd appreciate it if you could help move it forward when you have a chance. Thanks!

@mattip
mattip merged commit 2f93aec into numpy:main Jul 13, 2026
87 checks passed
@mattip

mattip commented Jul 13, 2026

Copy link
Copy Markdown
Member

Thanks @Ethan0456

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants