3

I have some code similar to the one below. That code leaks, and I don't know why. The thing that leaks is a simple creation of a Python class' instance inside a C code. The function I use to check the leak is create_n_times that's defined below and just creates new Python instances and derefrences them in a loop.

This is not an MWE per-se, but part of an example. To make it easier to understand, what the code does is:

  1. The Python code defines the dataclass and registers it into the C-extension using set_ip_settings_type.
  2. Then, a C-extension function create_n_times is called and that function creates and destroys n instances of the Python dataclass.

Can anyone help?

In Python:

import c_api

@dataclass
class IpSettings:
    ip: str
    port: int
    dhcp: bool

c_api.set_ip_settings_type(IpSettings)
c_api.generate_n_times(100000)

In C++ I have the following code that's compiled into a Python extension called c_api (it's a part of that library's definition):

#include <Python.h>

// ... Other functions including a "PyInit" function

extern "C" {
    
PyObject* ip_settings_type = NULL;
PyObject* set_ip_settings_type(PyObject* tp)
{
    Py_XDECREF(ip_settings_type);
    Py_INCREF(tp);
    ip_settings_type = tp;
    return Py_None;
}

PyObject* create_n_times(PyObject* n)
{
   long n_ = PyLong_AsLong(n);
   for (int i = 0; i < n_ ++i)
   {
       PyObject* factory_object = ip_settings_type;

       PyObject* args = PyTuple_New(3);
       PyTuple_SetItem(args, 0, PyUnicode_FromString("123.123.123.123"));
       PyTuple_SetItem(args, 1, PyLong_FromUnsignedLong(1231));
       PyTuple_SetItem(args, 2, Py_False);

       PyObject* obj = PyObject_CallObject(factory_object, args);
       Py_DECREF(obj);
   }

    return Py_None;
}

}
EZLearner
  • 1,614
  • 16
  • 25
  • 1
    Shouldn't there be a Py_DECREF(args) inside the for loop? – Neil May 04 '23 at 00:10
  • @Neil I thought so too, but when I add it I get memory-related errors... Let me check it again. – EZLearner May 04 '23 at 00:15
  • I don't understand, why the C++ tag? – Thomas Matthews May 04 '23 at 00:17
  • @Neil Yes... By adding `Py_DECREF(args);` after `Py_DECREF(obj);` I get the error `free(): invalid pointer`, but not right away - after over 10K iterations. – EZLearner May 04 '23 at 00:18
  • @ThomasMatthews It started as a C++ question, but I reduced it to C. It's more of Python-API question anyway. I'll remove that tag. – EZLearner May 04 '23 at 00:20
  • 1
    PyTuple_SetItem steals the reference to the supplied object, but Py_False is a single object. When the args tuple is destroyed, is the reference count for Py_False getting mangled? Try using PyBool_FromLong(0) to create a new reference to Py_False, like the other two SetItems. (see https://docs.python.org/3/c-api/bool.html) – Neil May 04 '23 at 00:39
  • @Neil Great job!!! This was the issue. Post it as an answer and I'll confirm it! For some reason I thought `Py_False` was a macro... That's the last time I listen to ChatGPT LOL (it generated the code). Thank you very much! – EZLearner May 04 '23 at 00:42
  • 1
    You might have the same issue with the reference counts for `return Py_None` (https://docs.python.org/3/c-api/none.html). It looks like it should use `Py_RETURN_NONE` instead. – Neil May 04 '23 at 00:43
  • Yeah, In my code I actually use `Py_INCREF` on `Py_None` before returning it, but you are correct. That might also be possible for `Py_True`/`Py_False`. – EZLearner May 04 '23 at 00:45

1 Answers1

5

PyTuple_SetItem steals the reference to the supplied object, but Py_False is a single object. When the args tuple is destroyed, the reference count for Py_False isgetting mangled.

Use PyBool_FromLong(0) to create a new reference to Py_False, like the other two calls to PyTuple_SetItem. (see docs.python.org/3/c-api/bool.html)

Neil
  • 2,378
  • 1
  • 20
  • 28
  • 3
    ... or `Py_INCREF(Py_False)` before assigning it to the tuple should do it, too, and would be more idiomatic. Either way, this is just the sort of thing that should be well documented in the code, lest some future dev attempt to "fix" it. – John Bollinger May 04 '23 at 01:09