Skip to content

BUG: Fix reference leaks in ArrayMethod resolvers and scalar paths - #119

Open
SwayamInSync wants to merge 2 commits into
numpy:mainfrom
SwayamInSync:fix-resolver-descr-leaks-115
Open

BUG: Fix reference leaks in ArrayMethod resolvers and scalar paths#119
SwayamInSync wants to merge 2 commits into
numpy:mainfrom
SwayamInSync:fix-resolver-descr-leaks-115

Conversation

@SwayamInSync

Copy link
Copy Markdown
Member

Closes #115

The ArrayMethod resolvers acquired descriptors slot by slot and returned on the first failure, leaving the earlier slots holding references. They now NULL-initialize loop_descrs and fail through a shared quad_resolve_descrs_fail() helper that clears every acquired slot.

This covers the 17 resolvers with a failure path:

  • binary one-output, binary two-output and ldexp
  • comparison and comparison-reduce
  • matmul, including both NotImplementedError branches
  • logical unary, frexp, and the unary property resolver
  • the NumPy, unicode, bytes and StringDType cast resolvers

Five descriptor lookups were also unchecked and reported success with a NULL entry: the ldexp NPY_INTP, frexp NPY_INT32 and unary-property NPY_BOOL lookups, and PyArray_GetDefaultDescr() in both NumPy cast resolvers. These now fail through the same path.

The cast call sites are where this leaks today. convert_datatype.c and dtype_transfer.c neither zero-initialize loop_descrs nor release it on error, while the ufunc path in ufunc_object.c happens to clean up on our behalf. The contract puts it on the resolver either way.

Reference leaks elsewhere

Auditing the rest of the extension for the same class of bug turned up four
leaks:

  • as_integer_ratio() leaked the numerator and denominator on every call, since PyTuple_Pack() increfs its items. 20,000 calls leaked 40,002 blocks.
  • QuadPrecDType() built a QuadPrecision scalar only to read its backend off it and never released it: one object per dtype construction.
  • Mixed-backend scalar arithmetic took two references on the right operand and released one on the TypeError path.
  • init_quadprec_dtype() leaked the cast specs when registration failed.

And five failure paths that crash rather than raise:

  • QuadPrecision("\ud800") segfaulted: PyUnicode_AsUTF8() returns NULL for lone surrogates and the NULL reached the parser. It now raises UnicodeEncodeError, matching np.longdouble.
  • The quad-to-quad copy branch used QuadPrecision_raw_new() without a NULL check.
  • init_casts() caught int, so std::bad_alloc from new reached std::terminate instead of raising MemoryError.
  • A failed SLEEF lock allocation set MemoryError and carried on with a NULL lock. Module init now fails.
  • PyModule_AddObject() steals a reference that was never owned; replaced with PyModule_AddObjectRef().

The matmul scratch buffers, the StringDType allocator pairing, Dragon4's thread-local scratch and the ufunc registration paths were checked and are already correct.

Testing

No new tests. Every remaining resolver error path needs an allocation failure to reach, and the one branch reachable without OOM (matmul with a mismatched out= backend) is cleaned up by NumPy itself, so a refcount test would pass without the fix as well.

The leaks above were measured with sys.getallocatedblocks(): 200,000 iterations of as_integer_ratio() plus QuadPrecDType() went from 400,000 leaked blocks to 1. Existing suite passes (7819 passed, 4 skipped, 18 xfailed).

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.

BUG: ArrayMethod descriptor resolvers leak references on allocation failure

1 participant