public inbox for linux-gpio@vger.kernel.org
 help / color / mirror / Atom feed
From: Vincent Fazio <vfazio@gmail.com>
To: linux-gpio@vger.kernel.org
Cc: Vincent Fazio <vfazio@gmail.com>
Subject: [libgpiod][PATCH 9/9] bindings: python: tests: migrate the gpiosim module to multi-phase init
Date: Tue, 21 Apr 2026 20:20:41 -0500	[thread overview]
Message-ID: <20260422012041.39933-10-vfazio@gmail.com> (raw)
In-Reply-To: <20260422012041.39933-1-vfazio@gmail.com>

Single-phase initialization has been classified as legacy within CPython
documentation [0] with multi-phase being its successor [1].

As such, switch to the new methodology.

Switching requires converting `gpiosim.Chip` to be a heap-type [2] in
order to access the module's state [3].

[0]: https://docs.python.org/3/c-api/extension-modules.html#legacy-single-phase-initialization
[1]: https://docs.python.org/3/c-api/extension-modules.html#multi-phase-initialization
[2]: https://docs.python.org/3/c-api/typeobj.html#heap-types
[3]: https://docs.python.org/3/howto/isolating-extensions.html#heap-types

Signed-off-by: Vincent Fazio <vfazio@gmail.com>
---
 bindings/python/tests/gpiosim/ext.c | 151 +++++++++++++++++-----------
 1 file changed, 94 insertions(+), 57 deletions(-)

diff --git a/bindings/python/tests/gpiosim/ext.c b/bindings/python/tests/gpiosim/ext.c
index 1ebd5af..9954dd5 100644
--- a/bindings/python/tests/gpiosim/ext.c
+++ b/bindings/python/tests/gpiosim/ext.c
@@ -45,21 +45,6 @@ struct module_state {
 	struct gpiosim_ctx *sim_ctx;
 };
 
-static void free_module_state(void *mod)
-{
-	struct module_state *state = PyModule_GetState((PyObject *)mod);
-
-	if (state->sim_ctx)
-		gpiosim_ctx_unref(state->sim_ctx);
-}
-
-static PyModuleDef module_def = {
-	PyModuleDef_HEAD_INIT,
-	.m_name = "gpiosim._ext",
-	.m_size = sizeof(struct module_state),
-	.m_free = free_module_state,
-};
-
 typedef struct {
 	PyObject_HEAD
 	struct gpiosim_dev *dev;
@@ -71,13 +56,11 @@ static int chip_init(chip_object *self,
 		     PyObject *Py_UNUSED(ignored1))
 {
 	struct module_state *state;
-	PyObject *mod;
 
-	mod = PyState_FindModule(&module_def);
-	if (!mod)
-		return -1;
+	state = PyType_GetModuleState(Py_TYPE(self));
 
-	state = PyModule_GetState(mod);
+	if (!state)
+		return -1;
 
 	self->dev = gpiosim_dev_new(state->sim_ctx);
 	if (!self->dev) {
@@ -111,12 +94,14 @@ static void chip_finalize(chip_object *self)
 static void chip_dealloc(PyObject *self)
 {
 	int ret;
+	PyTypeObject *tp = Py_TYPE(self);
 
 	ret = PyObject_CallFinalizerFromDealloc(self);
 	if (ret < 0)
 		return;
 
 	PyObject_Free(self);
+	Py_DECREF(tp);
 }
 
 static PyObject *chip_dev_path(chip_object *self, void *Py_UNUSED(ignored))
@@ -289,58 +274,110 @@ static PyMethodDef chip_methods[] = {
 	{ }
 };
 
-static PyTypeObject chip_type = {
-	PyVarObject_HEAD_INIT(NULL, 0)
-	.tp_name = "gpiosim.Chip",
-	.tp_basicsize = sizeof(chip_object),
-	.tp_flags = Py_TPFLAGS_DEFAULT,
-	.tp_new = PyType_GenericNew,
-	.tp_init = (initproc)chip_init,
-	.tp_finalize = (destructor)chip_finalize,
-	.tp_dealloc = (destructor)chip_dealloc,
-	.tp_methods = chip_methods,
-	.tp_getset = chip_getset,
+static PyType_Slot chip_type_slots[] = {
+	{Py_tp_new, PyType_GenericNew},
+	{Py_tp_init, (initproc)chip_init},
+	{Py_tp_finalize, (destructor)chip_finalize},
+	{Py_tp_dealloc, (destructor)chip_dealloc},
+	{Py_tp_methods, chip_methods},
+	{Py_tp_getset, chip_getset},
+	{0, 0}
 };
 
-PyMODINIT_FUNC PyInit__ext(void)
+/*
+See xxlimited.c and _bz2module.c for inspiration.
+
+As part of transitioning to multi-phase module initialization, the
+gpiosim.Chip type needs to become heap allocated so that it can access
+module state.
+
+We disallow subclassing by not specifying Py_TPFLAGS_BASETYPE. This
+allows the module to use PyType_GetModuleState() since it may otherwise
+not return the proper module if a subclass is invoking the method.
+
+Note:
+We do not hold PyObject references so no reference cycles should exist. As such,
+we do not set Py_TPFLAGS_HAVE_GC nor define either tp_traverse or tp_clear.
+
+There is still some ongoing debate about this this use case however:
+  https://github.com/python/cpython/issues/116946
+
+Note:
+If we allow subclassing in the future, reconsider use of PyObject_Free vs using
+the function defined in the tp_free slot.
+
+See also:
+  https://github.com/python/cpython/pull/138329#issuecomment-3242079564
+  https://github.com/python/cpython/issues/116946#issuecomment-3242135537
+  https://github.com/python/cpython/pull/139073
+  https://github.com/python/cpython/commit/ec689187957cc80af56b9a63251bbc295bafd781
+*/
+static PyType_Spec chip_type_spec = {
+	.name = "gpiosim.Chip",
+	.basicsize = sizeof(chip_object),
+	.flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE),
+	.slots = chip_type_slots,
+};
+
+static int module_exec(PyObject *module)
 {
 	const struct module_const *modconst;
 	struct module_state *state;
-	PyObject *module;
+	PyObject *chip_type_obj;
 	int ret;
 
-	module = PyModule_Create(&module_def);
-	if (!module)
-		return NULL;
-
-	ret = PyState_AddModule(module, &module_def);
-	if (ret) {
-		Py_DECREF(module);
-		return NULL;
-	}
-
 	state = PyModule_GetState(module);
 
 	state->sim_ctx = gpiosim_ctx_new();
 	if (!state->sim_ctx) {
-		Py_DECREF(module);
-		return PyErr_SetFromErrno(PyExc_OSError);
+		PyErr_SetFromErrno(PyExc_OSError);
+		return -1;
 	}
 
-	ret = PyModule_AddType(module, &chip_type);
-	if (ret) {
-		Py_DECREF(module);
-		return NULL;
-	}
+	chip_type_obj = PyType_FromModuleAndSpec(module, &chip_type_spec, NULL);
+
+	if (!chip_type_obj)
+		return -1;
+
+	ret = PyModule_AddType(module, (PyTypeObject*)chip_type_obj);
+	Py_DECREF(chip_type_obj);
+	if (ret < 0)
+		return -1;
 
 	for (modconst = module_constants; modconst->name; modconst++) {
-		ret = PyModule_AddIntConstant(module,
-					      modconst->name, modconst->val);
-		if (ret) {
-			Py_DECREF(module);
-			return NULL;
-		}
+		ret = PyModule_AddIntConstant(module, modconst->name,
+					      modconst->val);
+		if (ret < 0)
+			return -1;
+	}
+
+	return 0;
+}
+
+static void free_module_state(void *mod)
+{
+	struct module_state *state = PyModule_GetState((PyObject *)mod);
+
+	if (state->sim_ctx) {
+		gpiosim_ctx_unref(state->sim_ctx);
+		state->sim_ctx = NULL;
 	}
+}
+
+static struct PyModuleDef_Slot module_slots[] = {
+	{Py_mod_exec, module_exec},
+	{0, NULL},
+};
+
+static PyModuleDef module_def = {
+	PyModuleDef_HEAD_INIT,
+	.m_name = "gpiosim._ext",
+	.m_size = sizeof(struct module_state),
+	.m_free = free_module_state,
+	.m_slots = module_slots,
+};
 
-	return module;
+PyMODINIT_FUNC PyInit__ext(void)
+{
+	return PyModuleDef_Init(&module_def);
 }
-- 
2.43.0


      parent reply	other threads:[~2026-04-22  1:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-22  1:20 [libgpiod][PATCH 0/9] bindings: python: modernize C extensions Vincent Fazio
2026-04-22  1:20 ` [libgpiod][PATCH 1/9] bindings: python: use Py_RETURN_NONE in chip_get_line_name Vincent Fazio
2026-04-22  1:20 ` [libgpiod][PATCH 2/9] bindings: python: avoid PyObject_CallMethod during chip finalize Vincent Fazio
2026-04-22  1:20 ` [libgpiod][PATCH 3/9] bindings: python: avoid PyObject_CallMethod during request finalize Vincent Fazio
2026-04-22  1:20 ` [libgpiod][PATCH 4/9] bindings: python: simplify disallowing _ext.Request from being created Vincent Fazio
2026-04-22  1:20 ` [libgpiod][PATCH 5/9] bindings: python: use suggestions from upgrade_pythoncapi.py Vincent Fazio
2026-04-22  1:20 ` [libgpiod][PATCH 6/9] bindings: python: use PyImport_ImportModuleAttrString when available Vincent Fazio
2026-04-22  1:20 ` [libgpiod][PATCH 7/9] bindings: python: migrate the gpiod._ext module to multi-phase init Vincent Fazio
2026-04-22  1:20 ` [libgpiod][PATCH 8/9] bindings: python: tests: migrate the system " Vincent Fazio
2026-04-22  1:20 ` Vincent Fazio [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260422012041.39933-10-vfazio@gmail.com \
    --to=vfazio@gmail.com \
    --cc=linux-gpio@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox