Avoid leaking module object on numpy C-API import failure - #263
Avoid leaking module object on numpy C-API import failure#263antonwolfy wants to merge 1 commit into
Conversation
import_array() and import_umath() are macros that expand to `return NULL;` on failure. When they were called after PyModule_Create(), a failing import would return directly out of PyInit__ufuncs without releasing the module object, leaking the strong reference created by PyModule_Create(). Move the imports before the module is created so there is no owned reference to leak when an import fails. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| /* import_array()/import_umath() expand to `return NULL;` on failure, so | ||
| * call them before creating the module object to avoid leaking it. */ |
There was a problem hiding this comment.
if these return NULL, why would the module object leak? It's not clear to me, we don't check if the imports are NULL anywhere either
There was a problem hiding this comment.
There will be leak (if any import_array()/import_umath() failed and call return NULL) if calls placed after m = PyModule_Create(&_ufuncs_module); since no Py_XDECREF(m)
There was a problem hiding this comment.
Oh I see, these functions are macros internally in NumPy which expand to 'return NULL' or 'return' respectively, so it actually crashes later or unexpectedly
There was a problem hiding this comment.
I assumed it was using PyImport_ImportModule. In that case, maybe make the comment a bit clearer for future reference
ndgrigorian
left a comment
ndgrigorian
left a comment
There was a problem hiding this comment.
other than nit LGTM
2 participants
Summary
import_array()andimport_umath()are macros that expand toreturn NULL;on failure. InPyInit__ufuncsthey were called afterPyModule_Create(), so a failing numpy C-API import returned directly out of the init function without releasing the module object — leaking the strong reference created byPyModule_Create().This moves the imports before the module is created, so there is no owned reference to leak when an import fails. This is the pattern documented for the numpy import macros, which are designed to be called from a context where a bare
return NULL;is valid.Notes
Latent, pre-existing bug; only triggers on the (rare, usually fatal) failure of numpy's C-API import at module init. No behavior change on the success path.