gh-132776: Cleanup for XIBufferViewType (gh-132821)

* add notes
* rename XIBufferViewObject to xibufferview
* move memoryview XIData code to memoryobject.c

Author: ericsnowcurrently

Include/internal/pycore_interp_structs.h

@@ -9,6 +9,7 @@ extern "C" {
 
 #include "pycore_ast_state.h"     // struct ast_state
 #include "pycore_llist.h"         // struct llist_node
+#include "pycore_memoryobject.h"  // struct _memoryobject_state
 #include "pycore_opcode_utils.h"  // NUM_COMMON_CONSTANTS
 #include "pycore_pymath.h"        // _PY_SHORT_FLOAT_REPR
 #include "pycore_structs.h"       // PyHamtObject
@@ -912,9 +913,10 @@ struct _is {
     struct _dtoa_state dtoa;
     struct _py_func_state func_state;
     struct _py_code_state code_state;
-
     struct _Py_dict_state dict_state;
     struct _Py_exc_state exc_state;
+    struct _memoryobject_state memobj_state;
+
     struct _Py_mem_interp_free_queue mem_free_queue;
 
     struct ast_state ast;

Include/internal/pycore_memoryobject.h

@@ -8,6 +8,17 @@ extern "C" {
 #  error "this header requires Py_BUILD_CORE define"
 #endif
 
+struct _memoryobject_state {
+    PyTypeObject *XIBufferViewType;
+};
+
+extern PyStatus _PyMemoryView_InitTypes(PyInterpreterState *);
+extern void _PyMemoryView_FiniTypes(PyInterpreterState *);
+
+// exported for _interpreters module
+PyAPI_FUNC(PyTypeObject *) _PyMemoryView_GetXIBuffewViewType(void);
+
+
 extern PyTypeObject _PyManagedBuffer_Type;
 
 PyObject *

Modules/_interpreters_common.h

@@ -5,6 +5,7 @@
     _RESOLVE_MODINIT_FUNC_NAME(NAME)
 
 
+#ifdef REGISTERS_HEAP_TYPES
 static int
 ensure_xid_class(PyTypeObject *cls, xidatafunc getdata)
 {
@@ -16,7 +17,6 @@ ensure_xid_class(PyTypeObject *cls, xidatafunc getdata)
     return _PyXIData_RegisterClass(&ctx, cls, getdata);
 }
 
-#ifdef REGISTERS_HEAP_TYPES
 static int
 clear_xid_class(PyTypeObject *cls)
 {

Modules/_interpretersmodule.c

@@ -9,6 +9,7 @@
 #include "pycore_code.h"          // _PyCode_HAS_EXECUTORS()
 #include "pycore_crossinterp.h"   // _PyXIData_t
 #include "pycore_interp.h"        // _PyInterpreterState_IDIncref()
+#include "pycore_memoryobject.h"  // _PyMemoryView_GetXIBuffewViewType()
 #include "pycore_modsupport.h"    // _PyArg_BadArgument()
 #include "pycore_namespace.h"     // _PyNamespace_New()
 #include "pycore_pybuffer.h"      // _PyBuffer_ReleaseInInterpreterAndRawFree()
@@ -36,23 +37,6 @@ _get_current_interp(void)
 #define look_up_interp _PyInterpreterState_LookUpIDObject
 
 
-static PyObject *
-_get_current_module(void)
-{
-    PyObject *name = PyUnicode_FromString(MODULE_NAME_STR);
-    if (name == NULL) {
-        return NULL;
-    }
-    PyObject *mod = PyImport_GetModule(name);
-    Py_DECREF(name);
-    if (mod == NULL) {
-        return NULL;
-    }
-    assert(mod != Py_None);
-    return mod;
-}
-
-
 static int
 is_running_main(PyInterpreterState *interp)
 {
@@ -71,204 +55,10 @@ is_running_main(PyInterpreterState *interp)
 }
 
 
-/* Cross-interpreter Buffer Views *******************************************/
-
-// XXX Release when the original interpreter is destroyed.
-
-typedef struct {
-    PyObject base;
-    Py_buffer *view;
-    int64_t interpid;
-} XIBufferViewObject;
-
-#define XIBufferViewObject_CAST(op) ((XIBufferViewObject *)(op))
-
-static PyObject *
-xibufferview_from_buffer(PyTypeObject *cls, Py_buffer *view, int64_t interpid)
-{
-    assert(interpid >= 0);
-
-    Py_buffer *copied = PyMem_RawMalloc(sizeof(Py_buffer));
-    if (copied == NULL) {
-        return NULL;
-    }
-    /* This steals the view->obj reference  */
-    *copied = *view;
-
-    XIBufferViewObject *self = PyObject_Malloc(sizeof(XIBufferViewObject));
-    if (self == NULL) {
-        PyMem_RawFree(copied);
-        return NULL;
-    }
-    PyObject_Init(&self->base, cls);
-    *self = (XIBufferViewObject){
-        .base = self->base,
-        .view = copied,
-        .interpid = interpid,
-    };
-    return (PyObject *)self;
-}
-
-static void
-xibufferview_dealloc(PyObject *op)
-{
-    XIBufferViewObject *self = XIBufferViewObject_CAST(op);
-
-    if (self->view != NULL) {
-        PyInterpreterState *interp =
-                        _PyInterpreterState_LookUpID(self->interpid);
-        if (interp == NULL) {
-            /* The interpreter is no longer alive. */
-            PyErr_Clear();
-            PyMem_RawFree(self->view);
-        }
-        else {
-            if (_PyBuffer_ReleaseInInterpreterAndRawFree(interp,
-                                                         self->view) < 0)
-            {
-                // XXX Emit a warning?
-                PyErr_Clear();
-            }
-        }
-    }
-
-    PyTypeObject *tp = Py_TYPE(self);
-    tp->tp_free(self);
-    /* "Instances of heap-allocated types hold a reference to their type."
-     * See: https://docs.python.org/3.11/howto/isolating-extensions.html#garbage-collection-protocol
-     * See: https://docs.python.org/3.11/c-api/typeobj.html#c.PyTypeObject.tp_traverse
-    */
-    // XXX Why don't we implement Py_TPFLAGS_HAVE_GC, e.g. Py_tp_traverse,
-    // like we do for _abc._abc_data?
-    Py_DECREF(tp);
-}
-
-static int
-xibufferview_getbuf(PyObject *op, Py_buffer *view, int flags)
-{
-    /* Only PyMemoryView_FromObject() should ever call this,
-       via _memoryview_from_xid() below. */
-    XIBufferViewObject *self = XIBufferViewObject_CAST(op);
-    *view = *self->view;
-    view->obj = op;
-    // XXX Should we leave it alone?
-    view->internal = NULL;
-    return 0;
-}
-
-static PyType_Slot XIBufferViewType_slots[] = {
-    {Py_tp_dealloc, xibufferview_dealloc},
-    {Py_bf_getbuffer, xibufferview_getbuf},
-    // We don't bother with Py_bf_releasebuffer since we don't need it.
-    {0, NULL},
-};
-
-static PyType_Spec XIBufferViewType_spec = {
-    .name = MODULE_NAME_STR ".CrossInterpreterBufferView",
-    .basicsize = sizeof(XIBufferViewObject),
-    .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE |
-              Py_TPFLAGS_DISALLOW_INSTANTIATION | Py_TPFLAGS_IMMUTABLETYPE),
-    .slots = XIBufferViewType_slots,
-};
-
-
-static PyTypeObject * _get_current_xibufferview_type(void);
-
-
-struct xibuffer {
-    Py_buffer view;
-    int used;
-};
-
-static PyObject *
-_memoryview_from_xid(_PyXIData_t *data)
-{
-    assert(_PyXIData_DATA(data) != NULL);
-    assert(_PyXIData_OBJ(data) == NULL);
-    assert(_PyXIData_INTERPID(data) >= 0);
-    struct xibuffer *view = (struct xibuffer *)_PyXIData_DATA(data);
-    assert(!view->used);
-
-    PyTypeObject *cls = _get_current_xibufferview_type();
-    if (cls == NULL) {
-        return NULL;
-    }
-
-    PyObject *obj = xibufferview_from_buffer(
-                        cls, &view->view, _PyXIData_INTERPID(data));
-    if (obj == NULL) {
-        return NULL;
-    }
-    PyObject *res = PyMemoryView_FromObject(obj);
-    if (res == NULL) {
-        Py_DECREF(obj);
-        return NULL;
-    }
-    view->used = 1;
-    return res;
-}
-
-static void
-_pybuffer_shared_free(void* data)
-{
-    struct xibuffer *view = (struct xibuffer *)data;
-    if (!view->used) {
-        PyBuffer_Release(&view->view);
-    }
-    PyMem_RawFree(data);
-}
-
-static int
-_pybuffer_shared(PyThreadState *tstate, PyObject *obj, _PyXIData_t *data)
-{
-    struct xibuffer *view = PyMem_RawMalloc(sizeof(struct xibuffer));
-    if (view == NULL) {
-        return -1;
-    }
-    view->used = 0;
-    if (PyObject_GetBuffer(obj, &view->view, PyBUF_FULL_RO) < 0) {
-        PyMem_RawFree(view);
-        return -1;
-    }
-    _PyXIData_Init(data, tstate->interp, view, NULL, _memoryview_from_xid);
-    data->free = _pybuffer_shared_free;
-    return 0;
-}
-
-static int
-register_memoryview_xid(PyObject *mod, PyTypeObject **p_state)
-{
-    // XIBufferView
-    assert(*p_state == NULL);
-    PyTypeObject *cls = (PyTypeObject *)PyType_FromModuleAndSpec(
-                mod, &XIBufferViewType_spec, NULL);
-    if (cls == NULL) {
-        return -1;
-    }
-    if (PyModule_AddType(mod, cls) < 0) {
-        Py_DECREF(cls);
-        return -1;
-    }
-    *p_state = cls;
-
-    // Register XID for the builtin memoryview type.
-    if (ensure_xid_class(&PyMemoryView_Type, _pybuffer_shared) < 0) {
-        return -1;
-    }
-    // We don't ever bother un-registering memoryview.
-
-    return 0;
-}
-
-
-
 /* module state *************************************************************/
 
 typedef struct {
     int _notused;
-
-    /* heap types */
-    PyTypeObject *XIBufferViewType;
 } module_state;
 
 static inline module_state *
@@ -280,51 +70,19 @@ get_module_state(PyObject *mod)
     return state;
 }
 
-static module_state *
-_get_current_module_state(void)
-{
-    PyObject *mod = _get_current_module();
-    if (mod == NULL) {
-        // XXX import it?
-        PyErr_SetString(PyExc_RuntimeError,
-                        MODULE_NAME_STR " module not imported yet");
-        return NULL;
-    }
-    module_state *state = get_module_state(mod);
-    Py_DECREF(mod);
-    return state;
-}
-
 static int
 traverse_module_state(module_state *state, visitproc visit, void *arg)
 {
-    /* heap types */
-    Py_VISIT(state->XIBufferViewType);
-
     return 0;
 }
 
 static int
 clear_module_state(module_state *state)
 {
-    /* heap types */
-    Py_CLEAR(state->XIBufferViewType);
-
     return 0;
 }
 
 
-static PyTypeObject *
-_get_current_xibufferview_type(void)
-{
-    module_state *state = _get_current_module_state();
-    if (state == NULL) {
-        return NULL;
-    }
-    return state->XIBufferViewType;
-}
-
-
 /* Python code **************************************************************/
 
 static const char *
@@ -1545,6 +1303,7 @@ module_exec(PyObject *mod)
 {
     PyInterpreterState *interp = PyInterpreterState_Get();
     module_state *state = get_module_state(mod);
+    (void)state;
 
     _PyXIData_lookup_context_t ctx;
     if (_PyXIData_GetLookupContext(interp, &ctx) < 0) {
@@ -1576,9 +1335,14 @@ module_exec(PyObject *mod)
         goto error;
     }
 
-    if (register_memoryview_xid(mod, &state->XIBufferViewType) < 0) {
+    PyTypeObject *XIBufferViewType = _PyMemoryView_GetXIBuffewViewType();
+    if (XIBufferViewType == NULL) {
         goto error;
     }
+    if (PyModule_AddType(mod, XIBufferViewType) < 0) {
+        Py_DECREF(XIBufferViewType);
+        return -1;
+    }
 
     return 0;