From 0ca6fc8c79b3cf3acb0cc72f58fb51b98cd52066 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 13 Nov 2025 14:38:25 -0700 Subject: [PATCH 1/8] apply critical sections and declare free-threaded support --- radix/_radix.c | 94 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 67 insertions(+), 27 deletions(-) diff --git a/radix/_radix.c b/radix/_radix.c index 6dd8b41..07736dc 100644 --- a/radix/_radix.c +++ b/radix/_radix.c @@ -36,6 +36,12 @@ # define Py_TYPE(ob) (((PyObject*)(ob))->ob_type) #endif +/* Critical sections are defined in the C API in Python 3.13 and newer */ +#if PY_VERSION_HEX < 0x030D0000 + # define Py_BEGIN_CRITICAL_SECTION(op) { + # define Py_END_CRITICAL_SECTION() } +#endif + /* Prototypes */ struct _RadixObject; struct _RadixIterObject; @@ -118,8 +124,9 @@ RadixNode_dealloc(RadixNodeObject *self) static PyObject * Radix_parent(RadixNodeObject *self, void *closure) { - PyObject *ret; + PyObject *ret = Py_None; radix_node_t *node; + Py_BEGIN_CRITICAL_SECTION(self); node = self->rn; /* walk up through parent to find parent node with data */ for ( ; ; ) @@ -130,14 +137,15 @@ Radix_parent(RadixNodeObject *self, void *closure) if (node->data) { ret = node->data; - Py_XINCREF(ret); - return ret; + break; } } else break; } - Py_RETURN_NONE; + Py_END_CRITICAL_SECTION(); + Py_XINCREF(ret); + return ret; } static PyMemberDef RadixNode_members[] = { {"data", T_OBJECT, offsetof(RadixNodeObject, user_attr), READONLY}, @@ -361,7 +369,9 @@ Radix_add(RadixObject *self, PyObject *args, PyObject *kw_args) if ((prefix = args_to_prefix(NULL, addr, packed, packlen, prefixlen)) == NULL) return NULL; + Py_BEGIN_CRITICAL_SECTION((PyObject *)self); node_obj = create_add_node(self, prefix); + Py_END_CRITICAL_SECTION(); Deref_Prefix(prefix); return node_obj; @@ -383,27 +393,36 @@ Radix_delete(RadixObject *self, PyObject *args, PyObject *kw_args) char *addr = NULL, *packed = NULL; long prefixlen = -1; Py_ssize_t packlen = -1; + PyObject *ret = Py_None; if (!PyArg_ParseTupleAndKeywords(args, kw_args, "|zlz#:delete", keywords, &addr, &prefixlen, &packed, &packlen)) return NULL; if ((prefix = args_to_prefix(&lprefix, addr, packed, packlen, prefixlen)) == NULL) return NULL; + + Py_BEGIN_CRITICAL_SECTION(self); if ((node = radix_search_exact(self->rt, prefix)) == NULL) { PyErr_SetString(PyExc_KeyError, "no such address"); - return NULL; + ret = NULL; } - if (node->data != NULL) { - node_obj = node->data; - node_obj->rn = NULL; - Py_XDECREF(node_obj); + if (ret != NULL) { + if (node->data != NULL) { + node_obj = node->data; + node_obj->rn = NULL; + Py_XDECREF(node_obj); + } + + radix_remove(self->rt, node); } + Py_END_CRITICAL_SECTION(); - radix_remove(self->rt, node); + if (ret != NULL) { + self->gen_id++; + } - self->gen_id++; - Py_INCREF(Py_None); - return Py_None; + Py_XINCREF(ret); + return ret; } PyDoc_STRVAR(Radix_search_exact_doc, @@ -432,8 +451,9 @@ Radix_search_exact(RadixObject *self, PyObject *args, PyObject *kw_args) return NULL; if ((prefix = args_to_prefix(&lprefix, addr, packed, packlen, prefixlen)) == NULL) return NULL; - + Py_BEGIN_CRITICAL_SECTION(self); node = radix_search_exact(self->rt, prefix); + Py_END_CRITICAL_SECTION(); if (node == NULL || node->data == NULL) { Py_INCREF(Py_None); return Py_None; @@ -470,9 +490,10 @@ Radix_search_best(RadixObject *self, PyObject *args, PyObject *kw_args) return NULL; if ((prefix = args_to_prefix(&lprefix, addr, packed, packlen, prefixlen)) == NULL) return NULL; - - if ((node = radix_search_best(self->rt, prefix)) == NULL || - node->data == NULL) { + Py_BEGIN_CRITICAL_SECTION(self); + node = radix_search_best(self->rt, prefix); + Py_END_CRITICAL_SECTION(); + if (node == NULL || node->data == NULL) { Py_INCREF(Py_None); return Py_None; } @@ -508,9 +529,10 @@ Radix_search_worst(RadixObject *self, PyObject *args, PyObject *kw_args) return NULL; if ((prefix = args_to_prefix(&lprefix, addr, packed, packlen, prefixlen)) == NULL) return NULL; - - if ((node = radix_search_worst(self->rt, prefix)) == NULL || - node->data == NULL) { + Py_BEGIN_CRITICAL_SECTION(self); + node = radix_search_worst(self->rt, prefix); + Py_END_CRITICAL_SECTION(); + if (node == NULL || node->data == NULL) { Py_INCREF(Py_None); return Py_None; } @@ -555,7 +577,9 @@ Radix_search_covered(RadixObject *self, PyObject *args, PyObject *kw_args) if ((ret = PyList_New(0)) == NULL) return NULL; + Py_BEGIN_CRITICAL_SECTION(self); radix_search_covered(self->rt, prefix, add_node_to_list, ret, 1); + Py_END_CRITICAL_SECTION(); return (ret); } @@ -591,7 +615,9 @@ Radix_search_covering(RadixObject *self, PyObject *args, PyObject *kw_args) return NULL; } + Py_BEGIN_CRITICAL_SECTION(self); radix_search_covering(self->rt, prefix, add_node_to_list, ret); + Py_END_CRITICAL_SECTION(); return ret; } @@ -615,10 +641,12 @@ Radix_nodes(RadixObject *self, PyObject *args) if ((ret = PyList_New(0)) == NULL) return NULL; + Py_BEGIN_CRITICAL_SECTION(self); RADIX_TREE_WALK(self->rt, node) { if (node->data != NULL) PyList_Append(ret, (PyObject *)node->data); } RADIX_TREE_WALK_END; + Py_END_CRITICAL_SECTION(); return (ret); } @@ -642,12 +670,14 @@ Radix_prefixes(RadixObject *self, PyObject *args) if ((ret = PyList_New(0)) == NULL) return NULL; + Py_BEGIN_CRITICAL_SECTION(self); RADIX_TREE_WALK(self->rt, node) { if (node->data != NULL) { PyList_Append(ret, ((RadixNodeObject *)node->data)->prefix); } } RADIX_TREE_WALK_END; + Py_END_CRITICAL_SECTION(); return (ret); } @@ -767,19 +797,21 @@ static PyObject * RadixIter_iternext(RadixIterObject *self) { radix_node_t *node; - PyObject *ret; + PyObject *ret = Py_None; if (self->gen_id != self->parent->gen_id) { PyErr_SetString(PyExc_RuntimeWarning, "Radix tree modified during iteration"); return (NULL); } - + Py_BEGIN_CRITICAL_SECTION(self); again: if ((node = self->rn) == NULL) { /* We have walked both trees */ - if (self->af == AF_INET6) - return NULL; + if (self->af == AF_INET6) { + ret = NULL; + goto done; + } /* Otherwise reset and start walk of IPv6 tree */ self->sp = self->iterstack; self->rn = self->parent->rt->head_ipv6; @@ -801,13 +833,17 @@ RadixIter_iternext(RadixIterObject *self) if (node->prefix == NULL || node->data == NULL) goto again; + done: + Py_END_CRITICAL_SECTION(); - ret = node->data; - Py_INCREF(ret); + if (ret == Py_None) { + ret = node->data; + } + Py_XINCREF(ret); return (ret); } -PyDoc_STRVAR(RadixIter_doc, +PyDoc_STRVAR(RadixIter_doc, "Radix tree iterator"); static PyTypeObject RadixIter_Type = { @@ -994,6 +1030,10 @@ static PyObject *module_initialize(void) m = Py_InitModule3("_radix", radix_methods, module_doc); #endif +#if Py_GIL_DISABLED + PyUnstable_Module_SetGIL(m, Py_MOD_GIL_NOT_USED); +#endif + /* Stash the callable constructor for use in Radix.__reduce__ */ d = PyModule_GetDict(m); radix_constructor = PyDict_GetItemString(d, "Radix"); From dce1571db45b83893a2d88cd23264d954d360b9c Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 13 Nov 2025 14:44:12 -0700 Subject: [PATCH 2/8] add 3.14t CI --- .github/workflows/python-package.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 8cfdb97..2e3d078 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -31,6 +31,7 @@ jobs: - '3.12' - '3.13' - '3.14' + - '3.14t' - 'pypy3.9' - 'pypy3.10' steps: From a63918e5825aa12ea7e0e96ac55fd92e639af14c Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 13 Nov 2025 14:55:54 -0700 Subject: [PATCH 3/8] fix windows build --- radix/_radix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radix/_radix.c b/radix/_radix.c index 07736dc..693fe9d 100644 --- a/radix/_radix.c +++ b/radix/_radix.c @@ -833,7 +833,7 @@ RadixIter_iternext(RadixIterObject *self) if (node->prefix == NULL || node->data == NULL) goto again; - done: + done:; Py_END_CRITICAL_SECTION(); if (ret == Py_None) { From 78246eec269267c0590d49d85ee200a4b383af73 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 14 Nov 2025 15:26:51 -0700 Subject: [PATCH 4/8] lock gen_id mutation --- radix/_radix.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/radix/_radix.c b/radix/_radix.c index 693fe9d..a3466f1 100644 --- a/radix/_radix.c +++ b/radix/_radix.c @@ -415,12 +415,13 @@ Radix_delete(RadixObject *self, PyObject *args, PyObject *kw_args) radix_remove(self->rt, node); } - Py_END_CRITICAL_SECTION(); if (ret != NULL) { self->gen_id++; } + Py_END_CRITICAL_SECTION(); + Py_XINCREF(ret); return ret; } From 3ffa7a956f724c68cc4f766841937e70d5752867 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 14 Nov 2025 15:27:06 -0700 Subject: [PATCH 5/8] Add multithreaded test --- tests/test_multithreaded_radix.py | 85 +++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 tests/test_multithreaded_radix.py diff --git a/tests/test_multithreaded_radix.py b/tests/test_multithreaded_radix.py new file mode 100644 index 0000000..04d4408 --- /dev/null +++ b/tests/test_multithreaded_radix.py @@ -0,0 +1,85 @@ +import ipaddress +import random +import threading + +from concurrent.futures import ThreadPoolExecutor + +import radix + + +rand = random.Random(0x4d3d3d3) + + +def random_address(network): + host_bits = 32 - network.prefixlen + random_bits = rand.randint(0, 2**host_bits - 1) + random_ip_int = int(network.network_address) + random_bits + return ipaddress.IPv4Address(random_ip_int) + + +def random_network(): + # leave at least 4 bits of randomness for network addresses + masklen = rand.randint(0, 28) + network_int = rand.randint(0, 2**masklen - 1) + return ipaddress.IPv4Network((network_int << (32 - masklen), masklen)) + + +def chunks(seq, size): + return (set(seq[pos:pos + size]) for pos in range(0, len(seq), size)) + + +def test_multithreaded_radix(): + networks = [random_network() for _ in range(100)] + address_lists = [ + [random_address(network) for _ in range(10)] for network in networks] + # flatten + addresses = [ + str(address) for addresses in address_lists for address in addresses] + + n_writers = 2 + n_readers = 4 + n_threads = n_writers + n_readers + b = threading.Barrier(n_threads) + + random.shuffle(addresses) + reader_chunks = list(chunks(addresses, len(addresses)//n_readers)) + + random.shuffle(addresses) + writer_chunks = list(chunks(addresses, len(addresses)//n_writers)) + + r = radix.Radix() + writers_done = threading.Event() + + def writer(addresses): + b.wait() + for address in addresses: + r.add(address) + + def reader(id, addresses): + b.wait() + while not writers_done.is_set(): + for address in addresses: + node = r.search_exact(address) + if node and node.network == address: + node.data.setdefault('id', id) + + # spawn thread pools for readers and writers, joining readers + # after writers have finished filling the table. + # we want reads to happen concurrently with writes to encourage races + reader_futures = [] + with ThreadPoolExecutor(max_workers=n_readers) as reader_pool: + for i in range(n_readers): + reader_futures.append( + reader_pool.submit(reader, i, reader_chunks[i])) + + writer_futures = [] + with ThreadPoolExecutor(max_workers=n_writers) as writer_pool: + for i in range(n_writers): + writer_futures.append( + writer_pool.submit(writer, writer_chunks[i])) + writers_done.set() + + for node in r: + written_id = node.data.get('id', None) + if written_id: + assert node.network in reader_chunks[node.data['id']] From fad0c69fe53d6a30f3dd024e270ea7cb50351064 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 17 Nov 2025 11:10:29 -0700 Subject: [PATCH 6/8] Take ownership of nodes before exiting critical sections --- radix/_radix.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/radix/_radix.c b/radix/_radix.c index a3466f1..e58218d 100644 --- a/radix/_radix.c +++ b/radix/_radix.c @@ -143,8 +143,8 @@ Radix_parent(RadixNodeObject *self, void *closure) else break; } - Py_END_CRITICAL_SECTION(); Py_XINCREF(ret); + Py_END_CRITICAL_SECTION(); return ret; } static PyMemberDef RadixNode_members[] = { @@ -420,9 +420,8 @@ Radix_delete(RadixObject *self, PyObject *args, PyObject *kw_args) self->gen_id++; } - Py_END_CRITICAL_SECTION(); - Py_XINCREF(ret); + Py_END_CRITICAL_SECTION(); return ret; } @@ -454,13 +453,14 @@ Radix_search_exact(RadixObject *self, PyObject *args, PyObject *kw_args) return NULL; Py_BEGIN_CRITICAL_SECTION(self); node = radix_search_exact(self->rt, prefix); - Py_END_CRITICAL_SECTION(); if (node == NULL || node->data == NULL) { - Py_INCREF(Py_None); - return Py_None; + node_obj = Py_None; + } + else { + node_obj = node->data; } - node_obj = node->data; Py_XINCREF(node_obj); + Py_END_CRITICAL_SECTION(); return (PyObject *)node_obj; } @@ -493,13 +493,14 @@ Radix_search_best(RadixObject *self, PyObject *args, PyObject *kw_args) return NULL; Py_BEGIN_CRITICAL_SECTION(self); node = radix_search_best(self->rt, prefix); - Py_END_CRITICAL_SECTION(); if (node == NULL || node->data == NULL) { - Py_INCREF(Py_None); - return Py_None; + node_obj = Py_None; + } + else { + node_obj = node->data; } - node_obj = node->data; Py_XINCREF(node_obj); + Py_END_CRITICAL_SECTION(); return (PyObject *)node_obj; } @@ -532,13 +533,14 @@ Radix_search_worst(RadixObject *self, PyObject *args, PyObject *kw_args) return NULL; Py_BEGIN_CRITICAL_SECTION(self); node = radix_search_worst(self->rt, prefix); - Py_END_CRITICAL_SECTION(); if (node == NULL || node->data == NULL) { - Py_INCREF(Py_None); - return Py_None; + node_obj = Py_None; + } + else { + node_obj = node->data; } - node_obj = node->data; Py_XINCREF(node_obj); + Py_END_CRITICAL_SECTION(); return (PyObject *)node_obj; } @@ -834,13 +836,13 @@ RadixIter_iternext(RadixIterObject *self) if (node->prefix == NULL || node->data == NULL) goto again; - done:; - Py_END_CRITICAL_SECTION(); - + done: if (ret == Py_None) { ret = node->data; } Py_XINCREF(ret); + Py_END_CRITICAL_SECTION(); + return (ret); } From 66986ccced1bca51f7b696ae528d340e134f7533 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 17 Nov 2025 11:20:25 -0700 Subject: [PATCH 7/8] fix invalid pointer assignment error --- radix/_radix.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/radix/_radix.c b/radix/_radix.c index e58218d..89b257c 100644 --- a/radix/_radix.c +++ b/radix/_radix.c @@ -438,7 +438,7 @@ static PyObject * Radix_search_exact(RadixObject *self, PyObject *args, PyObject *kw_args) { radix_node_t *node; - RadixNodeObject *node_obj; + PyObject *node_obj; prefix_t lprefix, *prefix; static char *keywords[] = { "network", "masklen", "packed", NULL }; @@ -457,11 +457,11 @@ Radix_search_exact(RadixObject *self, PyObject *args, PyObject *kw_args) node_obj = Py_None; } else { - node_obj = node->data; + node_obj = (PyObject *)node->data; } Py_XINCREF(node_obj); Py_END_CRITICAL_SECTION(); - return (PyObject *)node_obj; + return node_obj; } PyDoc_STRVAR(Radix_search_best_doc, @@ -478,7 +478,7 @@ static PyObject * Radix_search_best(RadixObject *self, PyObject *args, PyObject *kw_args) { radix_node_t *node; - RadixNodeObject *node_obj; + PyObject *node_obj; prefix_t lprefix, *prefix; static char *keywords[] = { "network", "masklen", "packed", NULL }; @@ -497,11 +497,11 @@ Radix_search_best(RadixObject *self, PyObject *args, PyObject *kw_args) node_obj = Py_None; } else { - node_obj = node->data; + node_obj = (PyObject *)node->data; } Py_XINCREF(node_obj); Py_END_CRITICAL_SECTION(); - return (PyObject *)node_obj; + return node_obj; } PyDoc_STRVAR(Radix_search_worst_doc, @@ -518,7 +518,7 @@ static PyObject * Radix_search_worst(RadixObject *self, PyObject *args, PyObject *kw_args) { radix_node_t *node; - RadixNodeObject *node_obj; + PyObject *node_obj; prefix_t lprefix, *prefix; static char *keywords[] = { "network", "masklen", "packed", NULL }; @@ -537,11 +537,11 @@ Radix_search_worst(RadixObject *self, PyObject *args, PyObject *kw_args) node_obj = Py_None; } else { - node_obj = node->data; + node_obj = (PyObject *)node->data; } Py_XINCREF(node_obj); Py_END_CRITICAL_SECTION(); - return (PyObject *)node_obj; + return node_obj; } static int From 39ea5a296d62b849ef72ddc0ca97e999dd721123 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 4 Dec 2025 11:58:37 -0700 Subject: [PATCH 8/8] bump version --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index c518b53..d02c978 100755 --- a/setup.py +++ b/setup.py @@ -10,7 +10,7 @@ from os.path import abspath, dirname, join # specify the version -version = 'v1.0.5' +version = 'v1.1.0' here = abspath(dirname(__file__))