From 724f3fa0f94d2387b3d26dd44a8428a430461834 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 24 Jul 2025 10:21:02 +0300 Subject: [PATCH 1/3] Fix segfault due to list.pop by falling back to a generic method call --- mypyc/lib-rt/list_ops.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mypyc/lib-rt/list_ops.c b/mypyc/lib-rt/list_ops.c index 31a0d5cec7d5..4adecad1cac2 100644 --- a/mypyc/lib-rt/list_ops.c +++ b/mypyc/lib-rt/list_ops.c @@ -237,10 +237,16 @@ void CPyList_SetItemUnsafe(PyObject *list, Py_ssize_t index, PyObject *value) { PyObject *CPyList_PopLast(PyObject *obj) { +#ifdef Py_GIL_DISABLED + // The optimized version causes segfaults on a free-threaded Python 3.14b4 build, + // at least on macOS, so fall back to a generic implementation. + return PyObject_CallMethod(obj, "pop", NULL); +#else // I tried a specalized version of pop_impl for just removing the // last element and it wasn't any faster in microbenchmarks than // the generic one so I ditched it. return list_pop_impl((PyListObject *)obj, -1); +#endif } PyObject *CPyList_Pop(PyObject *obj, CPyTagged index) From 4dec7ec205fb2846abea2b52004ce3788598550e Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 24 Jul 2025 10:47:01 +0300 Subject: [PATCH 2/3] Update primitive for list.pop(n) --- mypyc/lib-rt/list_ops.c | 15 +++++++++++++++ mypyc/primitives/list_ops.py | 2 +- mypyc/test-data/run-lists.test | 29 +++++++++++++++++++++++------ 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/mypyc/lib-rt/list_ops.c b/mypyc/lib-rt/list_ops.c index 4adecad1cac2..cfded3f56154 100644 --- a/mypyc/lib-rt/list_ops.c +++ b/mypyc/lib-rt/list_ops.c @@ -253,7 +253,22 @@ PyObject *CPyList_Pop(PyObject *obj, CPyTagged index) { if (CPyTagged_CheckShort(index)) { Py_ssize_t n = CPyTagged_ShortAsSsize_t(index); +#ifdef Py_GIL_DISABLED + static PyObject *interned_pop_str = NULL; + if (!interned_pop_str) { + interned_pop_str = PyUnicode_InternFromString("pop"); + if (!interned_pop_str) + return NULL; + } + PyObject *index_obj = PyLong_FromLong(n); // New reference + if (index_obj == NULL) + return NULL; + PyObject *result = PyObject_CallMethodOneArg(obj, interned_pop_str, index_obj); + Py_DECREF(index_obj); + return result; +#else return list_pop_impl((PyListObject *)obj, n); +#endif } else { PyErr_SetString(PyExc_OverflowError, CPYTHON_LARGE_INT_ERRMSG); return NULL; diff --git a/mypyc/primitives/list_ops.py b/mypyc/primitives/list_ops.py index 516d9e1a4e02..b9d20a25bea3 100644 --- a/mypyc/primitives/list_ops.py +++ b/mypyc/primitives/list_ops.py @@ -219,7 +219,7 @@ ) # list.pop(index) -list_pop = method_op( +method_op( name="pop", arg_types=[list_rprimitive, int_rprimitive], return_type=object_rprimitive, diff --git a/mypyc/test-data/run-lists.test b/mypyc/test-data/run-lists.test index 03d5741b9eca..54bcc0384604 100644 --- a/mypyc/test-data/run-lists.test +++ b/mypyc/test-data/run-lists.test @@ -150,7 +150,9 @@ print(primes(13)) \[0, 0, 1, 1] \[0, 0, 1, 1, 0, 1, 0, 1, 0, 0, 0, 1, 0, 1] -[case testListBuild] +[case testListPrimitives] +from testutil import assertRaises + def test_list_build() -> None: # Currently LIST_BUILDING_EXPANSION_THRESHOLD equals to 10 # long list built by list_build_op @@ -169,9 +171,6 @@ def test_list_build() -> None: l3.append('a') assert l3 == ['a'] -[case testListPrims] -from typing import List - def test_append() -> None: l = [1, 2] l.append(10) @@ -189,10 +188,28 @@ def test_pop_last() -> None: def test_pop_index() -> None: l = [1, 2, 10, 3] - l.pop(2) + assert l.pop(2) == 10 assert l == [1, 2, 3] - l.pop(-2) + assert l.pop(-2) == 2 assert l == [1, 3] + assert l.pop(-2) == 1 + assert l.pop(0) == 3 + assert l == [] + l = [int() + 1000, int() + 1001, int() + 1002] + assert l.pop(0) == 1000 + assert l.pop(-1) == 1002 + assert l == [1001] + +def test_pop_index_errors() -> None: + l = [int() + 1000] + with assertRaises(IndexError): + l.pop(1) + with assertRaises(IndexError): + l.pop(-2) + with assertRaises(OverflowError): + l.pop(1 << 100) + with assertRaises(OverflowError): + l.pop(-(1 << 100)) def test_count() -> None: l = [1, 3] From 968a42aed8b3977af6e7d25ddbd87adb551686bb Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 28 Jul 2025 17:59:02 +0300 Subject: [PATCH 3/3] Use a faster implementation --- mypyc/lib-rt/list_ops.c | 43 ++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/mypyc/lib-rt/list_ops.c b/mypyc/lib-rt/list_ops.c index cfded3f56154..c611907fb601 100644 --- a/mypyc/lib-rt/list_ops.c +++ b/mypyc/lib-rt/list_ops.c @@ -235,17 +235,35 @@ void CPyList_SetItemUnsafe(PyObject *list, Py_ssize_t index, PyObject *value) { PyList_SET_ITEM(list, index, value); } -PyObject *CPyList_PopLast(PyObject *obj) +#ifdef Py_GIL_DISABLED +// The original optimized list.pop implementation doesn't work on free-threaded +// builds, so provide an alternative that is a bit slower but works. +// +// Note that this implementation isn't intended to be atomic. +static inline PyObject *list_pop_index(PyObject *list, Py_ssize_t index) { + PyObject *item = PyList_GetItemRef(list, index); + if (item == NULL) { + return NULL; + } + if (PySequence_DelItem(list, index) < 0) { + Py_DECREF(item); + return NULL; + } + return item; +} +#endif + +PyObject *CPyList_PopLast(PyObject *list) { #ifdef Py_GIL_DISABLED - // The optimized version causes segfaults on a free-threaded Python 3.14b4 build, - // at least on macOS, so fall back to a generic implementation. - return PyObject_CallMethod(obj, "pop", NULL); + // The other implementation causes segfaults on a free-threaded Python 3.14b4 build. + Py_ssize_t index = PyList_GET_SIZE(list) - 1; + return list_pop_index(list, index); #else // I tried a specalized version of pop_impl for just removing the // last element and it wasn't any faster in microbenchmarks than // the generic one so I ditched it. - return list_pop_impl((PyListObject *)obj, -1); + return list_pop_impl((PyListObject *)list, -1); #endif } @@ -254,18 +272,11 @@ PyObject *CPyList_Pop(PyObject *obj, CPyTagged index) if (CPyTagged_CheckShort(index)) { Py_ssize_t n = CPyTagged_ShortAsSsize_t(index); #ifdef Py_GIL_DISABLED - static PyObject *interned_pop_str = NULL; - if (!interned_pop_str) { - interned_pop_str = PyUnicode_InternFromString("pop"); - if (!interned_pop_str) - return NULL; + // We must use a slower implementation on free-threaded builds. + if (n < 0) { + n += PyList_GET_SIZE(obj); } - PyObject *index_obj = PyLong_FromLong(n); // New reference - if (index_obj == NULL) - return NULL; - PyObject *result = PyObject_CallMethodOneArg(obj, interned_pop_str, index_obj); - Py_DECREF(index_obj); - return result; + return list_pop_index(obj, n); #else return list_pop_impl((PyListObject *)obj, n); #endif