Skip to content

Refleak in PyGen_New()? Also, PyGen_New() is not tested #99485

Closed

Description

@vstinner

The public C API PyGen_New() is implemented by calling gen_new_with_qualname().

gen_new_with_qualname() code evolved in Python 3.11:

-    Py_INCREF(f->f_code);
-    gen->gi_code = (PyObject *)(f->f_code);
+    gen->gi_code = PyFrame_GetCode(f);
     gen->gi_code = PyFrame_GetCode(f);
+    Py_INCREF(gen->gi_code);
+    Py_DECREF(f);

I'm surprised by the added Py_INCREF(gen->gi_code); since PyFrame_GetCode() already returns a strong reference. For me, it's a reference leak.

I tried but failed to write an unit test on the PyGen_New() function. I wrote the test:

static PyObject *
test_gen_new(PyObject *self, PyObject *Py_UNUSED(args))
{
    PyCodeObject *code = PyCode_NewEmpty("filename", "func_name", 1);
    if (code == NULL) {
        return NULL;
    }

    PyObject *globals = PyDict_New();
    if (globals == NULL) {
        return NULL;
    }
    PyObject *locals = globals;  // borrowed ref

    PyThreadState *tstate = PyThreadState_Get();
    PyFrameObject *frame = PyFrame_New(tstate, code, globals, locals);
    if (frame == NULL) {
        return NULL;
    }

    PyObject *gen = PyGen_New(frame);
    Py_DECREF(gen);

    Py_DECREF(code);
    Py_DECREF(globals);
    Py_DECREF(frame);
    Py_RETURN_NONE;
}

But when I run it, it fails with an assertion error:

python: Objects/genobject.c:937: gen_new_with_qualname: Assertion `frame->frame_obj == f' failed.

Extract of the code. I added newlines and "HERE" comment around the code causing the assertion error:

    /* Copy the frame */
    assert(f->f_frame->frame_obj == NULL);
    assert(f->f_frame->owner == FRAME_OWNED_BY_FRAME_OBJECT);
    _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe;
    _PyFrame_Copy((_PyInterpreterFrame *)f->_f_frame_data, frame);
    gen->gi_frame_state = FRAME_CREATED;

    assert(frame->frame_obj == f); // <=== HERE

    f->f_frame = frame;
    frame->owner = FRAME_OWNED_BY_GENERATOR;
    assert(PyObject_GC_IsTracked((PyObject *)f));
    gen->gi_code = PyFrame_GetCode(f);
    Py_DECREF(f);

But the problem doesn't come from _PyFrame_Copy(). If I added the following assertion, it also fails:

assert(f->f_frame->frame_obj == f);

I am not sure if PyFrame_New() is supposed to create a frame with frame->f_frame->frame_obj == frame or not.

Linked PRs

Activity

  1. vstinner commented on Nov 14, 2022

    @vstinner
    MemberAuthor

    I created PR #99486 which adds the test which fails with the assertion error.

  2. vstinner commented on Nov 14, 2022

    @vstinner
    MemberAuthor

    @markshannon @brandtbucher @iritkatriel: Do you have any clue why the assertion fails? Is it expected? Do I misuse PyGen_New() API?

    PyGen_New() documentation doesn't explain how the frame should be created: https://docs.python.org/dev/c-api/gen.html#c.PyGen_New

  3. sobolevn commented on Nov 14, 2022

    @sobolevn
    Member

    @vstinner thanks for raising this, I was not able to write a test case for this as well in #94808

    Right now this API seems broken, untested, and undocumented :(

    Things I've tried:

    • Using frames from existing functions / generator functions
    • Using new frames created via C API
    • Following old examples from times when PyGen_New was used
  4. vstinner commented on Nov 14, 2022

    @vstinner
    MemberAuthor

    I also failed to find usages of PyGen_New() in the wild (3rd party code). Maybe only CPython used it in the past, but the code was reworked and CPython no longer uses it neither. Maybe it should be deprecated and scheduled from removal? There are similar functions:

    • PyGen_NewWithQualName()
    • PyGen_New()
    • PyCoro_New()
    • PyAsyncGen_New()

    These 4 functions take a frame as argument and call internally the private function gen_new_with_qualname().

  5. vstinner commented on Dec 7, 2022

    @vstinner
    MemberAuthor

    I cannot find any user of these 4 functions in the PyPI top 5000 projects:

    $ ./search_pypi_top.py PYPI-2020-09-01/ 'PyGen_New|PyGen_NewWithQualName|PyCoro_New|PyAsyncGen_New' -q -o PyGen_New
    PYPI-2020-09-01/Cython-0.29.32.tar.gz: Cython-0.29.32/Cython/Includes/cpython/genobject.pxd: object PyGen_New(PyFrameObject *frame)
    PYPI-2020-09-01/Cython-0.29.32.tar.gz: Cython-0.29.32/Cython/Includes/cpython/genobject.pxd: object PyGen_NewWithQualName(PyFrameObject *frame, object name, object qualname)
    
    Found 2 matching lines in 1 projects
    
  6. vstinner commented on Dec 7, 2022

    @vstinner
    MemberAuthor

    @markshannon: With a small hack, I managed to use PyGen_New() C API in Python 3.10.

    I added this code to the _testcapi extension:

    static PyObject*
    test_gen(PyObject *Py_UNUSED(self), PyObject *args)
    {
        PyCodeObject *code;
        PyObject *ns;
        if (!PyArg_ParseTuple(args, "O!O", &PyCode_Type, &code, &ns)) {
            return NULL;
        }
    
        PyThreadState *tstate = PyThreadState_Get();
        PyFrameObject *frame = PyFrame_New(tstate, code, ns, ns);
        // PyGen_New() requires a fresh frame
        frame->f_back = NULL;
    
        PyObject *gen = PyGen_New(Py_NewRef(frame));
        Py_DECREF(frame);
        return gen;
    }

    Usage of the C function in Python:

    import _testcapi
    
    def gen_code():
        yield 1
        yield 2
        yield 3
    
    gen = _testcapi.test_gen(gen_code.__code__, globals())
    for item in gen:
        print("item", item)

    Output:

    item 1
    item 2
    item 3
    

    In Python 3.10, PyFrame_New() has the comment: /* Legacy API */ but it's not deprecated (no Py_DEPRECATED(3.10) in the header file).

    The PyFrame_New() function is not documented, I don't know if it's supposed to be used :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    type-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions