[Python-Dev] Highlighting reference-stealing APIs [was Re: cpython: Fix email post-commit review comments.]

[Sam Partington]

On 19 April 2012 02:20, Nick Coghlan <ncoghlan at gmail.com> wrote:
> On Thu, Apr 19, 2012 at 12:21 AM, Antoine Pitrou <solipsis at pitrou.net> wrote:
>> (and here we see why reference-stealing APIs are a nuisance: because
>> you never know in advance whether a function will steal a reference or
>> not, and you have to read the docs for each and every C API call you
>> make)
>
> Yeah, it would have been nice if there was an explicit hint in the API
> names when reference stealing was involved, but I guess it's far too
> late now :(

It's too late to change the fn names sure, but you could change the
argument names in question for reference stealing apis with some kind
of markup.

That would make it fairly easy to write a script that did the checking for you :

int PyTuple_SetItem(PyObject *p, Py_ssize_t pos, PyObject *stolen_o)

Or better yet would be to mark the types :

int PyTuple_SetItem(PyObject *p, Py_ssize_t pos, PyStolenObject* o)
PyBorrowedObject* PyTuple_GetItem(PyObject *p, Py_ssize_t pos)

PyStolenObject and PyBorrowedObject would just be typedefs to PyObject
normally. But a consenting user could define PyENABLE_CHECKED_REFS
before including Python.h which would given

#if defined(PyENABLE_CHECKED_STOLEN_REFS)
struct PyStolenObject;
struct PyBorrowedObject;
#define PyYesIKnowItsStolen(o) ((PyStolenObject*)o)
#define PyYesIKnowItsBorrowed(o) ((PyObject*)o)
#else
typedef PyStolenObject PyObject;
typedef PyBorrowedObject PyObject;
#endif

Forcing the user to use

PyTuple_SetItem(p, pos, PyYesIKnowItsStolen(o))
PyObject * ref = PyYesIKnowItsBorrowed(PyTuple_GetItem(p, pos));

Or else it would fail to compile.  The user could even add her own :

PyStolenObject* IncRefBecauseIKnowItsStolen(PyObject* o) {
PyIncRef(o); return (PyStolenObject*)o; }
PyObject* IncRefBecauseIKnowItsBorrowed(PyBorrowedObject* o) {
PyIncRef(o); return (PyObject*)o; }

This would not require a special gcc build and would be available to
anyone who wanted it. I use a similar, C++ based trick in my python
extension code to avoid the whole issue of ref leaking, but I have to
be careful at the point of calling the python api, having it automatic
would be great.

On a similar note, I have just implemented a wrapper around Python.h
which runtime checks that the GIL is held around every call to the
Python API or else fails very noisily. This was done because it turns
out that wxPython had a ton of non-GIL calls to the API causing random
sporadic set faults in our app.  We now use it on several of our
extensions.  It doesn't require any changes to Python.h,  you just
need to add an include path before the python include path. Would
there be any interest in this?

Sam


On 19 April 2012 02:25, Nick Coghlan <ncoghlan at gmail.com> wrote:
> On Thu, Apr 19, 2012 at 11:04 AM, Gregory P. Smith <greg at krypto.org> wrote:
>> +1  Adding these annotations and setting up a buildbot that builds using
>> cpychecker would be a great.
>
> Even without the extra annotations, running cpychecker on at least one
> of the buildbots might be helpful.
>
> I'm in the process of setting up a buildbot for RHEL 6, once I get it
> up and running normally, I'll look into what it would take to install
> and enable cpychecker for the builds. (Or, alternatively, I may make
> it a separate cron job, similar to the daily refcount leak detection
> run).
>
> Cheers,
> Nick.
>
> --
> Nick Coghlan   |   ncoghlan at gmail.com   |   Brisbane, Australia
> _______________________________________________
> Python-Dev mailing list
> Python-Dev at python.org
> http://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe: http://mail.python.org/mailman/options/python-dev/sam.partington%40gmail.com