[issue22453] PyObject_REPR macro causes refcount leak
Serhiy Storchaka added the comment: Thank you for your report Chris. PyObject_REPR() is used only in Python/compile.c just before calling Py_FatalError(). So refcount leaks doesn't matter in these cases. PyObject_REPR() is expanded to deprecated _PyUnicode_AsString which is not defined if Py_LIMITED_API is defined. So it is unlikely that third-party code uses it. We can just remove this macro in 3.5. There are other bugs in formatting fatal error messages where PyObject_REPR() is used. PyBytes_AS_STRING() is called for unicode objects. Definitely this branch of the code is rarely executed. Here is a patch which expands PyObject_REPR() in Python/compile.c and replaces PyBytes_AS_STRING() with PyUnicode_AsUTF8(). In 3.5 we also should remove the PyObject_REPR macro definition. -- assignee: - serhiy.storchaka keywords: +patch nosy: +serhiy.storchaka stage: - patch review type: behavior - resource usage versions: -Python 3.1, Python 3.2, Python 3.3 Added file: http://bugs.python.org/file37220/PyObject_REPR.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22453] PyObject_REPR macro causes refcount leak
STINNER Victor added the comment: PyObject_REPR() is expanded to deprecated _PyUnicode_AsString which is not defined if Py_LIMITED_API is defined. PyObject_REPR() is still defined if Py_LIMITED_API is defined, I just checked. -- nosy: +haypo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22453] PyObject_REPR macro causes refcount leak
Serhiy Storchaka added the comment: PyObject_REPR() is still defined if Py_LIMITED_API is defined, I just checked. But it is expanded to undefined name. So it is not usable in any case. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22453] PyObject_REPR macro causes refcount leak
STINNER Victor added the comment: PyObject_REPR.patch: the first part looks good to me. For the second part, you can use PySys_FormatStderr() which is more complex but more correct: it formats the string as Unicode and then encode it to stderr encoding. PyUnicode_FromFormatV() is probably safer to handle errors. You may use PySys_FormatStderr() in the two functions to write context, and then call Py_FatalError with a simple message. The exact formatting doesn't matter much, these cases must never occur :-) An assertion may be enough :-p -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22453] PyObject_REPR macro causes refcount leak
STINNER Victor added the comment: Serhiy Storchaka wrote: But it is expanded to undefined name. So it is not usable in any case. Ah correct, I didn't notice _PyUnicode_AsString in the expanded result (I checked with gcc -E). When Py_LIMITED_API is set, PyObject_REPR(o) is expanded to _PyUnicode_AsString(PyObject_Repr(o)). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22453] PyObject_REPR macro causes refcount leak
Antoine Pitrou added the comment: PyObject_REPR() is expanded to deprecated _PyUnicode_AsString which is not defined if Py_LIMITED_API is defined. So it is unlikely that third-party code uses it Py_LIMITED_API is the stable ABI. Most third-party code doesn't use it, so it may still use PyObject_REPR(). -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22453] PyObject_REPR macro causes refcount leak
Serhiy Storchaka added the comment: For the second part, you can use PySys_FormatStderr() which is more complex but more correct PySys_FormatStderr() is more heavy function, it depends on working sys.stderr and codecs. Taking into account the lack of tests we should be careful with such changes. So I prefer to keep fprintf. Py_LIMITED_API is the stable ABI. Most third-party code doesn't use it, so it may still use PyObject_REPR(). So we should just add a warning? This macro is not documented anywhere. -/* Helper for passing objects to printf and the like */ -#define PyObject_REPR(obj) _PyUnicode_AsString(PyObject_Repr(obj)) +#ifndef Py_LIMITED_API +/* Helper for passing objects to printf and the like. + Leaks refcounts. Don't use it! +*/ +#define PyObject_REPR(obj) PyUnicode_AsUTF8(PyObject_Repr(obj)) +#endif -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22453] PyObject_REPR macro causes refcount leak
Antoine Pitrou added the comment: Le 18/11/2014 17:39, Serhiy Storchaka a écrit : Py_LIMITED_API is the stable ABI. Most third-party code doesn't use it, so it may still use PyObject_REPR(). So we should just add a warning? This macro is not documented anywhere. Well we can still remove it, and add a porting note in whatsnew for 3.5. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22453] PyObject_REPR macro causes refcount leak
Serhiy Storchaka added the comment: Here is updated patch. -- Added file: http://bugs.python.org/file37221/PyObject_REPR_2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___diff -r 0f663e0ce1d3 Doc/whatsnew/3.5.rst --- a/Doc/whatsnew/3.5.rst Tue Nov 18 17:30:50 2014 +0200 +++ b/Doc/whatsnew/3.5.rst Tue Nov 18 19:14:09 2014 +0200 @@ -441,3 +441,7 @@ Changes in the C API * The :c:type:`PyMemAllocator` structure was renamed to :c:type:`PyMemAllocatorEx` and a new ``calloc`` field was added. + +* Removed non-documented macro :c:macro:`PyObject_REPR` which leaked references. + Use format character ``%R`` in :c:func:`PyUnicode_FromFormat`-like functions + to format the :func:`repr` of the object. diff -r 0f663e0ce1d3 Include/object.h --- a/Include/object.h Tue Nov 18 17:30:50 2014 +0200 +++ b/Include/object.h Tue Nov 18 19:14:09 2014 +0200 @@ -575,9 +575,6 @@ PyAPI_FUNC(PyObject *) PyObject_Dir(PyOb PyAPI_FUNC(int) Py_ReprEnter(PyObject *); PyAPI_FUNC(void) Py_ReprLeave(PyObject *); -/* Helper for passing objects to printf and the like */ -#define PyObject_REPR(obj) _PyUnicode_AsString(PyObject_Repr(obj)) - /* Flag bits for printing: */ #define Py_PRINT_RAW1 /* No string quotes etc. */ diff -r 0f663e0ce1d3 Python/compile.c --- a/Python/compile.c Tue Nov 18 17:30:50 2014 +0200 +++ b/Python/compile.c Tue Nov 18 19:14:09 2014 +0200 @@ -1414,12 +1414,12 @@ get_ref_type(struct compiler *c, PyObjec PyOS_snprintf(buf, sizeof(buf), unknown scope for %.100s in %.100s(%s)\n symbols: %s\nlocals: %s\nglobals: %s, - PyBytes_AS_STRING(name), - PyBytes_AS_STRING(c-u-u_name), - PyObject_REPR(c-u-u_ste-ste_id), - PyObject_REPR(c-u-u_ste-ste_symbols), - PyObject_REPR(c-u-u_varnames), - PyObject_REPR(c-u-u_names) + PyUnicode_AsUTF8(name), + PyUnicode_AsUTF8(c-u-u_name), + PyUnicode_AsUTF8(PyObject_Repr(c-u-u_ste-ste_id)), + PyUnicode_AsUTF8(PyObject_Repr(c-u-u_ste-ste_symbols)), + PyUnicode_AsUTF8(PyObject_Repr(c-u-u_varnames)), + PyUnicode_AsUTF8(PyObject_Repr(c-u-u_names)) ); Py_FatalError(buf); } @@ -1476,11 +1476,11 @@ compiler_make_closure(struct compiler *c fprintf(stderr, lookup %s in %s %d %d\n freevars of %s: %s\n, -PyObject_REPR(name), -PyBytes_AS_STRING(c-u-u_name), +PyUnicode_AsUTF8(PyObject_Repr(name)), +PyUnicode_AsUTF8(c-u-u_name), reftype, arg, -_PyUnicode_AsString(co-co_name), -PyObject_REPR(co-co_freevars)); +PyUnicode_AsUTF8(co-co_name), +PyUnicode_AsUTF8(PyObject_Repr(co-co_freevars))); Py_FatalError(compiler_make_closure()); } ADDOP_I(c, LOAD_CLOSURE, arg); ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22453] PyObject_REPR macro causes refcount leak
STINNER Victor added the comment: PyObject_REPR_2.patch looks good to me, but it should only be applied to Python 3.5. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22453] PyObject_REPR macro causes refcount leak
Roundup Robot added the comment: New changeset e339d75a21d5 by Serhiy Storchaka in branch 'default': Issue #22453: Removed non-documented macro PyObject_REPR(). https://hg.python.org/cpython/rev/e339d75a21d5 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22453] PyObject_REPR macro causes refcount leak
Roundup Robot added the comment: New changeset 342a619cdafb by Serhiy Storchaka in branch '3.4': Issue #22453: Warn against the use of leaking macro PyObject_REPR(). https://hg.python.org/cpython/rev/342a619cdafb New changeset 6e26b5291c41 by Serhiy Storchaka in branch '2.7': Issue #22453: Fexed reference leaks when format error messages in ceval.c. https://hg.python.org/cpython/rev/6e26b5291c41 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22453] PyObject_REPR macro causes refcount leak
Serhiy Storchaka added the comment: Thank you for your reviews Victor and Antoine. -- resolution: - fixed stage: patch review - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22453] PyObject_REPR macro causes refcount leak
New submission from Chris Colbert: This is how the macro is defined in object.h: 2.7 /* Helper for passing objects to printf and the like */ #define PyObject_REPR(obj) PyString_AS_STRING(PyObject_Repr(obj)) 3.4 /* Helper for passing objects to printf and the like */ #define PyObject_REPR(obj) _PyUnicode_AsString(PyObject_Repr(obj)) PyObject_Repr returns a new reference, which is not released by the macro. This macro only seems to be used internally for error reporting in compile.c, so it's unlikely to be causing any pressing issues for the interpreter, but it may be biting some extension modules. -- components: Extension Modules, Interpreter Core messages: 227219 nosy: Chris.Colbert priority: normal severity: normal status: open title: PyObject_REPR macro causes refcount leak type: behavior versions: Python 2.7, Python 3.1, Python 3.2, Python 3.3, Python 3.4, Python 3.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22453 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
