BUG: cannot modify tuple after use #7965
Conversation
|
this bug show up on PyPy, CPython happily modifies the tuple |
| @@ -3694,6 +3693,7 @@ _vec_string_with_args(PyArrayObject* char_array, PyArray_Descr* type, | |||
| if (item_result == NULL) { | |||
| goto err; | |||
| } | |||
| Py_DECREF(args_tuple); | |||
There was a problem hiding this comment.
Shouldn't this happen before the if? If not I think we will be leaking a reference if the function call errors.
There was a problem hiding this comment.
Actually the Py_XDECREF(args_tuple); on line 3718 catches the error conditions, and moving the Py_DECREF up will, when there is an error, decref a NULL value. This is why I didn't move the declaration of arg_tuple into the loop - it would break the error handling
There was a problem hiding this comment.
Ah, had not looked at the err tag! Still, you are decreasing the reference count of args_tuple, not of item_result, so I'm pretty sure you can Py_DECREF before the if without fear: args_tuple has been checked to not be NULL before the function call, and should remain so after it, regardless of whether it fails or suceeds.
I think it's nicer to handle the variable inside the loop where it is needed, i.e. move this up and remove the Py_XDECREF call, but if you have strong objections to that, I'm open to discussion.
|
Mostly LGTM, just a couple of nitpicks. Thanks! |
| while (PyArray_MultiIter_NOTDONE(in_iter)) { | ||
| PyObject* item_result; | ||
| PyObject * args_tuple = PyTuple_New(n); |
There was a problem hiding this comment.
Is the space before * intentional? Sorry, greenhorn intervening.
|
Thanks a lot Matti, in it goes! |
Move PyTuple_New() into the loop since each use must reinitialize the tuple