| msg144330 - (view) |
Author: Suman Saha (Suman.Saha) |
Date: 2011-09-20 14:48 |
| Something that is allocated using PyTuple_Pack is not freed on one error path. |
|
|
| msg144363 - (view) |
Author: Meador Inge (meador.inge) *  |
Date: 2011-09-21 02:38 |
| I'll take this one. Suman, thanks for finding this. It will help in the future if you don't open a ton of bugs with the *exact* same title. They are harder to filter and keep track of that way. |
|
|
| msg144397 - (view) |
Author: Meador Inge (meador.inge) * |
Date: 2011-09-22 02:30 |
| This patch seems reasonable. Upon looking at the code in more detail, however, I can't see how the error condition that leaks the reference can ever by triggered. In particular, the following code from the 'PyCArrayType_from_ctype' function: if (!PyType_Check(itemtype)) { PyErr_SetString(PyExc_TypeError, "Expected a type object"); return NULL; } 'PyCArrayType_from_ctype' is only called from 'CDataType_repeat'. The 'CDataType_repeat' function is only used to implement the 'sq_repeat' slot in 'CDataType_as_sequence'. 'CDataType_as_sequence' is used in all of the implemented ctypes (struct, array, union, simple, ...). Unless 'PyCArrayType_from_ctype' is called through some other means (it is public), then 'itemtype' should *always* be a type. Or am I missing something obvious? So, we could add the decref or just remove the type check code all together (and make 'PyCArrayType_from_ctype' private). |
|
|
| msg144472 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2011-09-23 19:36 |
| My impression is that plugging refleaks (unlike minor speedups) is a bugfix rather than feature addition, so this and the other issues should be marked for 2.7 and 3.2 also. (I am only changing this one.) Deprecating a public (but obscure) CAPI function is a separate issue that would only affect 3.3 at the earliest (with a PendingDeprecation or Deprecation warning) and would be in addition to plugging the potential leak in the existing code. |
|
|
| msg144493 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2011-09-24 08:29 |
| I vote for plugging the refleak and keeping the function non-static. |
|
|
| msg144535 - (view) |
Author: Meador Inge (meador.inge) * |
Date: 2011-09-26 01:01 |
| OK, I will just fix the ref leak in 2.7, 3.2, and 3.3. This is a pretty low-risk fix (as I mentioned before, I can't see how the error condition is even executed). |
|
|
| msg144539 - (view) |
Author: Ezio Melotti (ezio.melotti) * |
Date: 2011-09-26 14:31 |
| If the function is public I guess that some external module might use it, and possibly pass a wrong argument that triggers the leak. |
|
|
| msg144540 - (view) |
Author: Meador Inge (meador.inge) * |
Date: 2011-09-26 14:41 |
| > If the function is public I guess that some external module > might use it Agreed; That is the only case I could deduce as well, which I hinted at in . So, I will leave the error check and keep the function public for now. I will commit the ref leak fix today. |
|
|
| msg144560 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2011-09-28 01:57 |
| New changeset 1fb5b0cc6367 by Meador Inge in branch '2.7': Issue #13013: ctypes: Fix a reference leak in PyCArrayType_from_ctype. http://hg.python.org/cpython/rev/1fb5b0cc6367 New changeset 58a75eeb5c8e by Meador Inge in branch '3.2': Issue #13013: ctypes: Fix a reference leak in PyCArrayType_from_ctype. http://hg.python.org/cpython/rev/58a75eeb5c8e New changeset 1726fa560112 by Meador Inge in branch 'default': Issue #13013: ctypes: Fix a reference leak in PyCArrayType_from_ctype. http://hg.python.org/cpython/rev/1726fa560112 |
|
|