BUG: decref in failure path; replace PyObject_Type by Py_TYPE#11336
BUG: decref in failure path; replace PyObject_Type by Py_TYPE#11336eric-wieser merged 1 commit intonumpy:masterfrom
PyObject_Type by Py_TYPE#11336Conversation
PyObject_Type increases the reference count, which was not taken into account.
a84fff2 to
e8db634
Compare
|
@eric-wieser - if you have a chance, could you have a quick look at this one? I'm a bit worried about stacking too many PRs on top of each other... |
| for (i = 0; i < num_override_args; i++) { | ||
| obj = with_override[i]; | ||
| if (obj == NULL) { | ||
| override_obj = with_override[i]; |
There was a problem hiding this comment.
Can you move the declaration of override_obj within this loop too?
There was a problem hiding this comment.
Nevermind, that's not possible
| PyObject *other_obj = with_override[j]; | ||
| if (other_obj != NULL && | ||
| PyObject_Type(other_obj) != PyObject_Type(obj) && | ||
| Py_TYPE(other_obj) != Py_TYPE(override_obj) && |
There was a problem hiding this comment.
A nit: There's no point doing this extra check - it's just the first line of PyObject_IsInstance anyway.
Not going to make you change that in this PR though
There was a problem hiding this comment.
Surely, the first line is for object == other in isinstance, while here we do not want to defer to other if the types are the same. But I think you are right that the check can be removed, since we now explicitly remove instances of the same type...
There was a problem hiding this comment.
Nevermind, what I said above was wrong - it's not an optimization, it's a check for a strict subclass
PyObject_Type by Py_TYPE
PyObject_Typeincreases the reference count, which was not taken into account.Split off from #11320, since that one was getting too complex (don't understand the failure in USE_DEBUG...), and this part should probably be back-ported.