Skip to content

Improve guidelines for GC protocol implementation for heap types #138292

@picnixz

Description

@picnixz

This is a follow-up to #125962 which added some guidelines:

  • Use type->tp_alloc instead of PyObject_New and PyObject_GC_New.
  • Use type->tp_free instead of PyObject_Free and PyObject_GC_Del.

Those two recommendations were introduced to facilitate adding the Py_TPFLAGS_HAVE_GC flag to a heap type (those types must (should?) implement the GC protocol, at least according to the docs: https://docs.python.org/3/c-api/gcsupport.html#supporting-cycle-detection).

Now, the docs should indicate that:

What I am actually worried about is:

Constructors for container types must conform to two rules:

Now, tp_alloc automatically calls PyObject_GC_Track so users won't be able to pre-initialize fields, so I suggest that we mention this.

Outdated discussion

If people need to first initialize fields, maybe we should recommend constructing them first:

static PyObject *
object_new(PyTypeObject *type)
{
    T *self = NULL;
    PyObject *f1, *f2, *f3;
    
    f1 = do1();
    if (f1 == NULL) { goto error_pre_init; }
    f2 = do2();
    if (f2 == NULL) { goto error_pre_init; }
    f3 = do3();
    if (f3 == NULL) { goto error_pre_init; }
    
    self = (T *)type->tp_alloc(type, 0);
    if (self == NULL) {
        goto error_pre_init;
    }
    self->f1 = f1;
    self->f2 = f2;
    self->f3 = f3;
    f1 = f2 = f3 = NULL;

    if (finalize(self) < 0) {
        goto error;
    }
    return (PyObject *)self;

error_pre_init:
    Py_XDECREF(f1);
    Py_XDECREF(f2);
    Py_XDECREF(f3);
    return NULL;

error_post_init:
    Py_DECREF(self);
    return NULL;
}

cc @ZeroIntensity

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    Status

    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions