gh-132775: Cleanup Related to crossinterp.c Before Further Changes (gh-132974)

This change consists of adding tests and moving code around, with some renaming thrown in.

1 files changed, 107 insertions, 97 deletions

diff --git a/Python/crossinterp.c b/Python/crossinterp.c
index e4368165a35..8ba88c4b057 100644
--- a/Python/crossinterp.c
+++ b/Python/crossinterp.c
@@ -74,7 +74,7 @@ static xidatafunc lookup_getdata(struct _dlcontext *, PyObject *);
 _PyXIData_t *
 _PyXIData_New(void)
 {
-    _PyXIData_t *xid = PyMem_RawMalloc(sizeof(_PyXIData_t));
+    _PyXIData_t *xid = PyMem_RawCalloc(1, sizeof(_PyXIData_t));
     if (xid == NULL) {
         PyErr_NoMemory();
     }
@@ -93,58 +93,58 @@ _PyXIData_Free(_PyXIData_t *xid)
 /* defining cross-interpreter data */
 
 static inline void
-_xidata_init(_PyXIData_t *data)
+_xidata_init(_PyXIData_t *xidata)
 {
     // If the value is being reused
     // then _xidata_clear() should have been called already.
-    assert(data->data == NULL);
-    assert(data->obj == NULL);
-    *data = (_PyXIData_t){0};
-    _PyXIData_INTERPID(data) = -1;
+    assert(xidata->data == NULL);
+    assert(xidata->obj == NULL);
+    *xidata = (_PyXIData_t){0};
+    _PyXIData_INTERPID(xidata) = -1;
 }
 
 static inline void
-_xidata_clear(_PyXIData_t *data)
+_xidata_clear(_PyXIData_t *xidata)
 {
     // _PyXIData_t only has two members that need to be
-    // cleaned up, if set: "data" must be freed and "obj" must be decref'ed.
+    // cleaned up, if set: "xidata" must be freed and "obj" must be decref'ed.
     // In both cases the original (owning) interpreter must be used,
     // which is the caller's responsibility to ensure.
-    if (data->data != NULL) {
-        if (data->free != NULL) {
-            data->free(data->data);
+    if (xidata->data != NULL) {
+        if (xidata->free != NULL) {
+            xidata->free(xidata->data);
         }
-        data->data = NULL;
+        xidata->data = NULL;
     }
-    Py_CLEAR(data->obj);
+    Py_CLEAR(xidata->obj);
 }
 
 void
-_PyXIData_Init(_PyXIData_t *data,
+_PyXIData_Init(_PyXIData_t *xidata,
                PyInterpreterState *interp,
                void *shared, PyObject *obj,
                xid_newobjfunc new_object)
 {
-    assert(data != NULL);
+    assert(xidata != NULL);
     assert(new_object != NULL);
-    _xidata_init(data);
-    data->data = shared;
+    _xidata_init(xidata);
+    xidata->data = shared;
     if (obj != NULL) {
         assert(interp != NULL);
         // released in _PyXIData_Clear()
-        data->obj = Py_NewRef(obj);
+        xidata->obj = Py_NewRef(obj);
     }
     // Ideally every object would know its owning interpreter.
     // Until then, we have to rely on the caller to identify it
     // (but we don't need it in all cases).
-    _PyXIData_INTERPID(data) = (interp != NULL)
+    _PyXIData_INTERPID(xidata) = (interp != NULL)
         ? PyInterpreterState_GetID(interp)
         : -1;
-    data->new_object = new_object;
+    xidata->new_object = new_object;
 }
 
 int
-_PyXIData_InitWithSize(_PyXIData_t *data,
+_PyXIData_InitWithSize(_PyXIData_t *xidata,
                        PyInterpreterState *interp,
                        const size_t size, PyObject *obj,
                        xid_newobjfunc new_object)
@@ -153,50 +153,28 @@ _PyXIData_InitWithSize(_PyXIData_t *data,
     // For now we always free the shared data in the same interpreter
     // where it was allocated, so the interpreter is required.
     assert(interp != NULL);
-    _PyXIData_Init(data, interp, NULL, obj, new_object);
-    data->data = PyMem_RawMalloc(size);
-    if (data->data == NULL) {
+    _PyXIData_Init(xidata, interp, NULL, obj, new_object);
+    xidata->data = PyMem_RawMalloc(size);
+    if (xidata->data == NULL) {
         return -1;
     }
-    data->free = PyMem_RawFree;
+    xidata->free = PyMem_RawFree;
     return 0;
 }
 
 void
-_PyXIData_Clear(PyInterpreterState *interp, _PyXIData_t *data)
+_PyXIData_Clear(PyInterpreterState *interp, _PyXIData_t *xidata)
 {
-    assert(data != NULL);
+    assert(xidata != NULL);
     // This must be called in the owning interpreter.
     assert(interp == NULL
-           || _PyXIData_INTERPID(data) == -1
-           || _PyXIData_INTERPID(data) == PyInterpreterState_GetID(interp));
-    _xidata_clear(data);
+           || _PyXIData_INTERPID(xidata) == -1
+           || _PyXIData_INTERPID(xidata) == PyInterpreterState_GetID(interp));
+    _xidata_clear(xidata);
 }
 
 
-/* using cross-interpreter data */
-
-static int
-_check_xidata(PyThreadState *tstate, _PyXIData_t *data)
-{
-    // data->data can be anything, including NULL, so we don't check it.
-
-    // data->obj may be NULL, so we don't check it.
-
-    if (_PyXIData_INTERPID(data) < 0) {
-        PyErr_SetString(PyExc_SystemError, "missing interp");
-        return -1;
-    }
-
-    if (data->new_object == NULL) {
-        PyErr_SetString(PyExc_SystemError, "missing new_object func");
-        return -1;
-    }
-
-    // data->free may be NULL, so we don't check it.
-
-    return 0;
-}
+/* getting cross-interpreter data */
 
 static inline void
 _set_xid_lookup_failure(PyThreadState *tstate, PyObject *obj, const char *msg,
@@ -216,6 +194,7 @@ _set_xid_lookup_failure(PyThreadState *tstate, PyObject *obj, const char *msg,
     }
 }
 
+
 int
 _PyObject_CheckXIData(PyThreadState *tstate, PyObject *obj)
 {
@@ -233,15 +212,39 @@ _PyObject_CheckXIData(PyThreadState *tstate, PyObject *obj)
     return 0;
 }
 
+static int
+_check_xidata(PyThreadState *tstate, _PyXIData_t *xidata)
+{
+    // xidata->data can be anything, including NULL, so we don't check it.
+
+    // xidata->obj may be NULL, so we don't check it.
+
+    if (_PyXIData_INTERPID(xidata) < 0) {
+        PyErr_SetString(PyExc_SystemError, "missing interp");
+        return -1;
+    }
+
+    if (xidata->new_object == NULL) {
+        PyErr_SetString(PyExc_SystemError, "missing new_object func");
+        return -1;
+    }
+
+    // xidata->free may be NULL, so we don't check it.
+
+    return 0;
+}
+
 int
 _PyObject_GetXIData(PyThreadState *tstate,
-                    PyObject *obj, _PyXIData_t *data)
+                    PyObject *obj, _PyXIData_t *xidata)
 {
     PyInterpreterState *interp = tstate->interp;
 
-    // Reset data before re-populating.
-    *data = (_PyXIData_t){0};
-    _PyXIData_INTERPID(data) = -1;
+    assert(xidata->data == NULL);
+    assert(xidata->obj == NULL);
+    if (xidata->data != NULL || xidata->obj != NULL) {
+        _PyErr_SetString(tstate, PyExc_ValueError, "xidata not cleared");
+    }
 
     // Call the "getdata" func for the object.
     dlcontext_t ctx;
@@ -251,13 +254,18 @@ _PyObject_GetXIData(PyThreadState *tstate,
     Py_INCREF(obj);
     xidatafunc getdata = lookup_getdata(&ctx, obj);
     if (getdata == NULL) {
+        if (PyErr_Occurred()) {
+            Py_DECREF(obj);
+            return -1;
+        }
+        // Fall back to obj
         Py_DECREF(obj);
         if (!_PyErr_Occurred(tstate)) {
             _set_xid_lookup_failure(tstate, obj, NULL, NULL);
         }
         return -1;
     }
-    int res = getdata(tstate, obj, data);
+    int res = getdata(tstate, obj, xidata);
     Py_DECREF(obj);
     if (res != 0) {
         PyObject *cause = _PyErr_GetRaisedException(tstate);
@@ -268,19 +276,22 @@ _PyObject_GetXIData(PyThreadState *tstate,
     }
 
     // Fill in the blanks and validate the result.
-    _PyXIData_INTERPID(data) = PyInterpreterState_GetID(interp);
-    if (_check_xidata(tstate, data) != 0) {
-        (void)_PyXIData_Release(data);
+    _PyXIData_INTERPID(xidata) = PyInterpreterState_GetID(interp);
+    if (_check_xidata(tstate, xidata) != 0) {
+        (void)_PyXIData_Release(xidata);
         return -1;
     }
 
     return 0;
 }
 
+
+/* using cross-interpreter data */
+
 PyObject *
-_PyXIData_NewObject(_PyXIData_t *data)
+_PyXIData_NewObject(_PyXIData_t *xidata)
 {
-    return data->new_object(data);
+    return xidata->new_object(xidata);
 }
 
 static int
@@ -291,52 +302,52 @@ _call_clear_xidata(void *data)
 }
 
 static int
-_xidata_release(_PyXIData_t *data, int rawfree)
+_xidata_release(_PyXIData_t *xidata, int rawfree)
 {
-    if ((data->data == NULL || data->free == NULL) && data->obj == NULL) {
+    if ((xidata->data == NULL || xidata->free == NULL) && xidata->obj == NULL) {
         // Nothing to release!
         if (rawfree) {
-            PyMem_RawFree(data);
+            PyMem_RawFree(xidata);
         }
         else {
-            data->data = NULL;
+            xidata->data = NULL;
         }
         return 0;
     }
 
     // Switch to the original interpreter.
     PyInterpreterState *interp = _PyInterpreterState_LookUpID(
-                                        _PyXIData_INTERPID(data));
+                                        _PyXIData_INTERPID(xidata));
     if (interp == NULL) {
         // The interpreter was already destroyed.
         // This function shouldn't have been called.
         // XXX Someone leaked some memory...
         assert(PyErr_Occurred());
         if (rawfree) {
-            PyMem_RawFree(data);
+            PyMem_RawFree(xidata);
         }
         return -1;
     }
 
     // "Release" the data and/or the object.
     if (rawfree) {
-        return _Py_CallInInterpreterAndRawFree(interp, _call_clear_xidata, data);
+        return _Py_CallInInterpreterAndRawFree(interp, _call_clear_xidata, xidata);
     }
     else {
-        return _Py_CallInInterpreter(interp, _call_clear_xidata, data);
+        return _Py_CallInInterpreter(interp, _call_clear_xidata, xidata);
     }
 }
 
 int
-_PyXIData_Release(_PyXIData_t *data)
+_PyXIData_Release(_PyXIData_t *xidata)
 {
-    return _xidata_release(data, 0);
+    return _xidata_release(xidata, 0);
 }
 
 int
-_PyXIData_ReleaseAndRawFree(_PyXIData_t *data)
+_PyXIData_ReleaseAndRawFree(_PyXIData_t *xidata)
 {
-    return _xidata_release(data, 1);
+    return _xidata_release(xidata, 1);
 }
 
 
@@ -455,15 +466,15 @@ _format_TracebackException(PyObject *tbexc)
 
 
 static int
-_release_xid_data(_PyXIData_t *data, int rawfree)
+_release_xid_data(_PyXIData_t *xidata, int rawfree)
 {
     PyObject *exc = PyErr_GetRaisedException();
     int res = rawfree
-        ? _PyXIData_Release(data)
-        : _PyXIData_ReleaseAndRawFree(data);
+        ? _PyXIData_Release(xidata)
+        : _PyXIData_ReleaseAndRawFree(xidata);
     if (res < 0) {
         /* The owning interpreter is already destroyed. */
-        _PyXIData_Clear(NULL, data);
+        _PyXIData_Clear(NULL, xidata);
         // XXX Emit a warning?
         PyErr_Clear();
     }
@@ -1107,7 +1118,7 @@ _PyXI_ApplyError(_PyXI_error *error)
 
 typedef struct _sharednsitem {
     const char *name;
-    _PyXIData_t *data;
+    _PyXIData_t *xidata;
     // We could have a "PyXIData _data" field, so it would
     // be allocated as part of the item and avoid an extra allocation.
     // However, doing so adds a bunch of complexity because we must
@@ -1132,7 +1143,7 @@ _sharednsitem_init(_PyXI_namespace_item *item, PyObject *key)
         assert(!_sharednsitem_is_initialized(item));
         return -1;
     }
-    item->data = NULL;
+    item->xidata = NULL;
     assert(_sharednsitem_is_initialized(item));
     return 0;
 }
@@ -1140,11 +1151,11 @@ _sharednsitem_init(_PyXI_namespace_item *item, PyObject *key)
 static int
 _sharednsitem_has_value(_PyXI_namespace_item *item, int64_t *p_interpid)
 {
-    if (item->data == NULL) {
+    if (item->xidata == NULL) {
         return 0;
     }
     if (p_interpid != NULL) {
-        *p_interpid = _PyXIData_INTERPID(item->data);
+        *p_interpid = _PyXIData_INTERPID(item->xidata);
     }
     return 1;
 }
@@ -1153,16 +1164,15 @@ static int
 _sharednsitem_set_value(_PyXI_namespace_item *item, PyObject *value)
 {
     assert(_sharednsitem_is_initialized(item));
-    assert(item->data == NULL);
-    item->data = PyMem_RawMalloc(sizeof(_PyXIData_t));
-    if (item->data == NULL) {
-        PyErr_NoMemory();
+    assert(item->xidata == NULL);
+    item->xidata = _PyXIData_New();
+    if (item->xidata == NULL) {
         return -1;
     }
     PyThreadState *tstate = PyThreadState_Get();
-    if (_PyObject_GetXIData(tstate, value, item->data) != 0) {
-        PyMem_RawFree(item->data);
-        item->data = NULL;
+    if (_PyObject_GetXIData(tstate, value, item->xidata) != 0) {
+        PyMem_RawFree(item->xidata);
+        item->xidata = NULL;
         // The caller may want to propagate PyExc_NotShareableError
         // if currently switched between interpreters.
         return -1;
@@ -1173,11 +1183,11 @@ _sharednsitem_set_value(_PyXI_namespace_item *item, PyObject *value)
 static void
 _sharednsitem_clear_value(_PyXI_namespace_item *item)
 {
-    _PyXIData_t *data = item->data;
-    if (data != NULL) {
-        item->data = NULL;
+    _PyXIData_t *xidata = item->xidata;
+    if (xidata != NULL) {
+        item->xidata = NULL;
         int rawfree = 1;
-        (void)_release_xid_data(data, rawfree);
+        (void)_release_xid_data(xidata, rawfree);
     }
 }
 
@@ -1195,7 +1205,7 @@ static int
 _sharednsitem_copy_from_ns(struct _sharednsitem *item, PyObject *ns)
 {
     assert(item->name != NULL);
-    assert(item->data == NULL);
+    assert(item->xidata == NULL);
     PyObject *value = PyDict_GetItemString(ns, item->name);  // borrowed
     if (value == NULL) {
         if (PyErr_Occurred()) {
@@ -1218,8 +1228,8 @@ _sharednsitem_apply(_PyXI_namespace_item *item, PyObject *ns, PyObject *dflt)
         return -1;
     }
     PyObject *value;
-    if (item->data != NULL) {
-        value = _PyXIData_NewObject(item->data);
+    if (item->xidata != NULL) {
+        value = _PyXIData_NewObject(item->xidata);
         if (value == NULL) {
             Py_DECREF(name);
             return -1;