From 2f9a29a243be0be28865c4f0a25a7fe414ddfb80 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Fri, 24 Oct 2025 17:55:12 +0200 Subject: [PATCH 01/14] Add support for free-threading Implement thread-safety using Cython 3.1+ critical sections to protect Pool operations (alloc, free, realloc) from race conditions. Update dependencies and CI to support free-threaded builds. --- .github/workflows/cibuildwheel.yml | 101 +++++++++++++++-------------- .github/workflows/tests.yml | 3 +- README.md | 17 +++++ cymem/cymem.pyx | 55 +++++++++++----- pyproject.toml | 4 +- requirements.txt | 2 +- setup.py | 5 +- 7 files changed, 115 insertions(+), 72 deletions(-) diff --git a/.github/workflows/cibuildwheel.yml b/.github/workflows/cibuildwheel.yml index a8419b9..e3ee404 100644 --- a/.github/workflows/cibuildwheel.yml +++ b/.github/workflows/cibuildwheel.yml @@ -7,6 +7,9 @@ on: # ** matches 'zero or more of any character' - 'release-v[0-9]+.[0-9]+.[0-9]+**' - 'prerelease-v[0-9]+.[0-9]+.[0-9]+**' + # TODO: Remove this before upstream PR + pull_request: + types: [opened, synchronize, reopened, edited] jobs: build_wheels: name: Build wheels on ${{ matrix.os }} @@ -19,7 +22,7 @@ jobs: steps: - uses: actions/checkout@v4 - name: Build wheels - uses: pypa/cibuildwheel@v2.21.3 + uses: pypa/cibuildwheel@v3.2.1 env: CIBW_SOME_OPTION: value with: @@ -43,50 +46,52 @@ jobs: with: name: cibw-sdist path: dist/*.tar.gz - create_release: - needs: [build_wheels, build_sdist] - runs-on: ubuntu-latest - permissions: - contents: write - checks: write - actions: read - issues: read - packages: write - pull-requests: read - repository-projects: read - statuses: read - steps: - - name: Get the tag name and determine if it's a prerelease - id: get_tag_info - run: | - FULL_TAG=${GITHUB_REF#refs/tags/} - if [[ $FULL_TAG == release-* ]]; then - TAG_NAME=${FULL_TAG#release-} - IS_PRERELEASE=false - elif [[ $FULL_TAG == prerelease-* ]]; then - TAG_NAME=${FULL_TAG#prerelease-} - IS_PRERELEASE=true - else - echo "Tag does not match expected patterns" >&2 - exit 1 - fi - echo "FULL_TAG=$TAG_NAME" >> $GITHUB_ENV - echo "TAG_NAME=$TAG_NAME" >> $GITHUB_ENV - echo "IS_PRERELEASE=$IS_PRERELEASE" >> $GITHUB_ENV - - uses: actions/download-artifact@v4 - with: - # unpacks all CIBW artifacts into dist/ - pattern: cibw-* - path: dist - merge-multiple: true - - name: Create Draft Release - id: create_release - uses: softprops/action-gh-release@v2 - if: startsWith(github.ref, 'refs/tags/') - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - with: - name: ${{ env.TAG_NAME }} - draft: true - prerelease: ${{ env.IS_PRERELEASE }} - files: "./dist/*" + + # TODO: Uncomment this before upstream PR + # create_release: + # needs: [build_wheels, build_sdist] + # runs-on: ubuntu-latest + # permissions: + # contents: write + # checks: write + # actions: read + # issues: read + # packages: write + # pull-requests: read + # repository-projects: read + # statuses: read + # steps: + # - name: Get the tag name and determine if it's a prerelease + # id: get_tag_info + # run: | + # FULL_TAG=${GITHUB_REF#refs/tags/} + # if [[ $FULL_TAG == release-* ]]; then + # TAG_NAME=${FULL_TAG#release-} + # IS_PRERELEASE=false + # elif [[ $FULL_TAG == prerelease-* ]]; then + # TAG_NAME=${FULL_TAG#prerelease-} + # IS_PRERELEASE=true + # else + # echo "Tag does not match expected patterns" >&2 + # exit 1 + # fi + # echo "FULL_TAG=$TAG_NAME" >> $GITHUB_ENV + # echo "TAG_NAME=$TAG_NAME" >> $GITHUB_ENV + # echo "IS_PRERELEASE=$IS_PRERELEASE" >> $GITHUB_ENV + # - uses: actions/download-artifact@v4 + # with: + # # unpacks all CIBW artifacts into dist/ + # pattern: cibw-* + # path: dist + # merge-multiple: true + # - name: Create Draft Release + # id: create_release + # uses: softprops/action-gh-release@v2 + # if: startsWith(github.ref, 'refs/tags/') + # env: + # GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # with: + # name: ${{ env.TAG_NAME }} + # draft: true + # prerelease: ${{ env.IS_PRERELEASE }} + # files: "./dist/*" diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b73968c..ed4bca5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -19,7 +19,8 @@ env: jobs: tests: name: Test - if: github.repository_owner == 'explosion' + # TODO: Uncomment this before upstream PR + # if: github.repository_owner == 'explosion' strategy: fail-fast: false matrix: diff --git a/README.md b/README.md index 9e21837..68f0f87 100644 --- a/README.md +++ b/README.md @@ -195,3 +195,20 @@ objects, but we'd still like the laziness of the `Pool`. from cymem.cymem cimport Pool, WrapMalloc, WrapFree cdef Pool mem = Pool(WrapMalloc(priv_malloc), WrapFree(priv_free)) ``` + +## Thread Safety + +As of version 2.0.12, `cymem.Pool` is thread-safe when used with CPython 3.13+ +free-threaded builds (PEP 703). All operations on the Pool, including `alloc()`, +`free()`, and `realloc()`, can be safely called from multiple threads concurrently. + +**Key guarantees:** +- Multiple threads can safely call `alloc()`, `free()`, and `realloc()` on the + same Pool instance +- The Pool's internal bookkeeping (`addresses` dict and `size` accounting) is + protected from race conditions + +**Important notes:** +- Individual `Pool` instances are thread-safe, but you are still responsible + for proper synchronization when accessing the memory contents themselves +- Custom memory allocators need to be thread-safe themselves. diff --git a/cymem/cymem.pyx b/cymem/cymem.pyx index e2470fb..5084ce1 100644 --- a/cymem/cymem.pyx +++ b/cymem/cymem.pyx @@ -1,7 +1,7 @@ -# cython: embedsignature=True +# cython: embedsignature=True, freethreading_compatible=True +cimport cython from cpython.mem cimport PyMem_Malloc, PyMem_Free -from cpython.ref cimport Py_INCREF, Py_DECREF from libc.string cimport memset from libc.string cimport memcpy import warnings @@ -44,6 +44,10 @@ cdef class Pool: addresses (dict): The currently allocated addresses and their sizes. Read-only. pymalloc (PyMalloc): The allocator to use (default uses PyMem_Malloc). pyfree (PyFree): The free to use (default uses PyMem_Free). + + Thread-safety: + This class is thread-safe in CPython 3.13+ free-threaded builds. + All public methods can be safely called from multiple threads. """ def __cinit__(self, PyMalloc pymalloc=Default_Malloc, @@ -55,6 +59,8 @@ cdef class Pool: self.pyfree = pyfree def __dealloc__(self): + # No need for locking here since the methods will be unreachable + # by the time __dealloc__ is called cdef size_t addr if self.addresses is not None: for addr in self.addresses: @@ -73,8 +79,9 @@ cdef class Pool: if p == NULL: raise MemoryError("Error assigning %d bytes" % (number * elem_size)) memset(p, 0, number * elem_size) - self.addresses[p] = number * elem_size - self.size += number * elem_size + with cython.critical_section(self): + self.addresses[p] = number * elem_size + self.size += number * elem_size return p cdef void* realloc(self, void* p, size_t new_size) except NULL: @@ -82,19 +89,30 @@ cdef class Pool: a non-NULL pointer to the new block. new_size must be larger than the original. - If p is not in the Pool or new_size is 0, a MemoryError is raised. + If p is not in the Pool or new_size is 0, a ValueError is raised. """ - if p not in self.addresses: - raise ValueError("Pointer %d not found in Pool %s" % (p, self.addresses)) if new_size == 0: raise ValueError("Realloc requires new_size > 0") - assert new_size > self.addresses[p] - cdef void* new_ptr = self.alloc(1, new_size) - if new_ptr == NULL: - raise MemoryError("Error reallocating to %d bytes" % new_size) - memcpy(new_ptr, p, self.addresses[p]) - self.free(p) - self.addresses[new_ptr] = new_size + + cdef size_t old_size + cdef void* new_ptr + with cython.critical_section(self): + if p not in self.addresses: + raise ValueError("Pointer %d not found in Pool %s" % (p, self.addresses)) + old_size = self.addresses[p] + assert new_size > old_size + + new_ptr = self.pymalloc.malloc(new_size) + if new_ptr == NULL: + raise MemoryError("Error reallocating to %d bytes" % new_size) + + memset(new_ptr, 0, new_size) + memcpy(new_ptr, p, old_size) + self.size -= self.addresses.pop(p) + self.addresses[new_ptr] = new_size + self.size += new_size + + self.pyfree.free(p) return new_ptr cdef void free(self, void* p) except *: @@ -105,7 +123,8 @@ cdef class Pool: If p is not in Pool.addresses, a KeyError is raised. """ - self.size -= self.addresses.pop(p) + with cython.critical_section(self): + self.size -= self.addresses.pop(p) self.pyfree.free(p) def own_pyref(self, object py_ref): @@ -142,9 +161,9 @@ cdef class Address: raise MemoryError("Error assigning %d bytes" % number * elem_size) memset(self.ptr, 0, number * elem_size) - property addr: - def __get__(self): - return self.ptr + @property + def addr(self): + return self.ptr def __dealloc__(self): if self.ptr != NULL: diff --git a/pyproject.toml b/pyproject.toml index ab36d78..3e85038 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [build-system] requires = [ "setuptools", - "cython>=0.25", + "cython>=3.1", ] build-backend = "setuptools.build_meta" @@ -9,7 +9,7 @@ build-backend = "setuptools.build_meta" build = "*" skip = "pp* cp36* cp37* cp38*" test-skip = "" -free-threaded-support = false +enable = ["cpython-freethreading"] archs = ["native"] diff --git a/requirements.txt b/requirements.txt index 94694dc..ddeb607 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1,2 @@ -cython>=0.25 +cython>=3.1 pytest diff --git a/setup.py b/setup.py index df40294..3c28269 100755 --- a/setup.py +++ b/setup.py @@ -100,8 +100,8 @@ def setup_package(): version=about["__version__"], url=about["__uri__"], license=about["__license__"], - ext_modules=cythonize(ext_modules, language_level=2), - setup_requires=["cython>=0.25"], + ext_modules=cythonize(ext_modules), + setup_requires=["cython>=3.1"], classifiers=[ "Environment :: Console", "Intended Audience :: Developers", @@ -116,6 +116,7 @@ def setup_package(): "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3.13", + "Programming Language :: Python :: 3.14", "Topic :: Scientific/Engineering", ], cmdclass={"build_ext": build_ext_subclass}, From bf80e4ee556f144f0e70bdb8631edcd1a5acdf9f Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Tue, 28 Oct 2025 10:32:26 +0100 Subject: [PATCH 02/14] Address feedback --- README.md | 8 +++++--- cymem/cymem.pyx | 10 ++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 68f0f87..959d5ac 100644 --- a/README.md +++ b/README.md @@ -204,11 +204,13 @@ free-threaded builds (PEP 703). All operations on the Pool, including `alloc()`, **Key guarantees:** - Multiple threads can safely call `alloc()`, `free()`, and `realloc()` on the - same Pool instance + same `Pool` instance. - The Pool's internal bookkeeping (`addresses` dict and `size` accounting) is - protected from race conditions + protected from race conditions. Reading the internal state without + holding a lock on the `Pool` instance via a critical section is not + thread-safe. **Important notes:** - Individual `Pool` instances are thread-safe, but you are still responsible - for proper synchronization when accessing the memory contents themselves + for proper synchronization when accessing the memory contents themselves. - Custom memory allocators need to be thread-safe themselves. diff --git a/cymem/cymem.pyx b/cymem/cymem.pyx index 5084ce1..f8808c1 100644 --- a/cymem/cymem.pyx +++ b/cymem/cymem.pyx @@ -46,15 +46,20 @@ cdef class Pool: pyfree (PyFree): The free to use (default uses PyMem_Free). Thread-safety: - This class is thread-safe in CPython 3.13+ free-threaded builds. - All public methods can be safely called from multiple threads. + All public methods in this class can be safely called from multiple threads. """ def __cinit__(self, PyMalloc pymalloc=Default_Malloc, PyFree pyfree=Default_Free): + # size, addresses and refs are mutable. Changing more than one of them + # requires locking to prevent data races. Changing one of them by calling + # Python operations is okay without locking because most Python operations + # are atomic. self.size = 0 self.addresses = {} self.refs = [] + + # pymalloc and pyfree are immutable. self.pymalloc = pymalloc self.pyfree = pyfree @@ -128,6 +133,7 @@ cdef class Pool: self.pyfree.free(p) def own_pyref(self, object py_ref): + # Calling append here is atomic, no critical section needed. self.refs.append(py_ref) From bca07951cbc35d7b3f2555378ca14079384e6e4e Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Tue, 28 Oct 2025 13:08:25 +0100 Subject: [PATCH 03/14] Apply suggestion from @crusaderky Co-authored-by: Guido Imperiale --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 3c28269..4d6bad5 100755 --- a/setup.py +++ b/setup.py @@ -117,6 +117,7 @@ def setup_package(): "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3.13", "Programming Language :: Python :: 3.14", + "Programming Language :: Python :: Free Threading :: 2 - Beta", "Topic :: Scientific/Engineering", ], cmdclass={"build_ext": build_ext_subclass}, From b9e76ce5c3cbc6d33b3332d9097a882e74e9ea91 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Tue, 28 Oct 2025 13:14:18 +0100 Subject: [PATCH 04/14] Address feedback Co-authored-by: Guido Imperiale --- cymem/cymem.pyx | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/cymem/cymem.pyx b/cymem/cymem.pyx index f8808c1..4c0e172 100644 --- a/cymem/cymem.pyx +++ b/cymem/cymem.pyx @@ -94,28 +94,27 @@ cdef class Pool: a non-NULL pointer to the new block. new_size must be larger than the original. - If p is not in the Pool or new_size is 0, a ValueError is raised. + If p is not in the Pool or new_size isn't larger than the previous size, + a ValueError is raised. """ - if new_size == 0: - raise ValueError("Realloc requires new_size > 0") - cdef size_t old_size cdef void* new_ptr with cython.critical_section(self): if p not in self.addresses: raise ValueError("Pointer %d not found in Pool %s" % (p, self.addresses)) old_size = self.addresses[p] - assert new_size > old_size + if new_size <= old_size: + raise ValueError("Realloc requires new_size > previous size") new_ptr = self.pymalloc.malloc(new_size) if new_ptr == NULL: raise MemoryError("Error reallocating to %d bytes" % new_size) - memset(new_ptr, 0, new_size) memcpy(new_ptr, p, old_size) - self.size -= self.addresses.pop(p) + memset( new_ptr + old_size, 0, new_size - old_size) + self.size += new_size - old_size + del self.addresses[p] self.addresses[new_ptr] = new_size - self.size += new_size self.pyfree.free(p) return new_ptr From 46db9f047464a666d905a345641366bfcca47235 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Wed, 29 Oct 2025 11:15:07 +0100 Subject: [PATCH 05/14] Address more feedback --- README.md | 10 ++++++++-- cymem/cymem.pyx | 24 +++++++++++++----------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 959d5ac..6997a26 100644 --- a/README.md +++ b/README.md @@ -211,6 +211,12 @@ free-threaded builds (PEP 703). All operations on the Pool, including `alloc()`, thread-safe. **Important notes:** -- Individual `Pool` instances are thread-safe, but you are still responsible - for proper synchronization when accessing the memory contents themselves. +- Individual Pool instances are thread-safe, but you are still responsible for + proper synchronization when accessing the memory contents themselves. Note + that holding a lock on the Pool itself is typically not the right approach: + since malloc is thread-safe, memory allocated by separate calls to `alloc` + can be safely accessed concurrently without locking. You should only + synchronize access to specific memory regions that are being shared across + threads, using fine-grained locks appropriate to your use case rather than a + coarse-grained lock on the entire `Pool`. - Custom memory allocators need to be thread-safe themselves. diff --git a/cymem/cymem.pyx b/cymem/cymem.pyx index 4c0e172..91c81c1 100644 --- a/cymem/cymem.pyx +++ b/cymem/cymem.pyx @@ -51,10 +51,8 @@ cdef class Pool: def __cinit__(self, PyMalloc pymalloc=Default_Malloc, PyFree pyfree=Default_Free): - # size, addresses and refs are mutable. Changing more than one of them - # requires locking to prevent data races. Changing one of them by calling - # Python operations is okay without locking because most Python operations - # are atomic. + # size, addresses and refs are mutable. note that operations on + # dicts and lists are atomic. self.size = 0 self.addresses = {} self.refs = [] @@ -84,7 +82,7 @@ cdef class Pool: if p == NULL: raise MemoryError("Error assigning %d bytes" % (number * elem_size)) memset(p, 0, number * elem_size) - with cython.critical_section(self): + with cython.critical_section(self, self.addresses): self.addresses[p] = number * elem_size self.size += number * elem_size return p @@ -99,17 +97,21 @@ cdef class Pool: """ cdef size_t old_size cdef void* new_ptr - with cython.critical_section(self): + + new_ptr = self.pymalloc.malloc(new_size) + if new_ptr == NULL: + raise MemoryError("Error reallocating to %d bytes" % new_size) + + with cython.critical_section(self, self.addresses): if p not in self.addresses: + self.pyfree.free(new_ptr) raise ValueError("Pointer %d not found in Pool %s" % (p, self.addresses)) + old_size = self.addresses[p] if new_size <= old_size: + self.pyfree.free(new_ptr) raise ValueError("Realloc requires new_size > previous size") - new_ptr = self.pymalloc.malloc(new_size) - if new_ptr == NULL: - raise MemoryError("Error reallocating to %d bytes" % new_size) - memcpy(new_ptr, p, old_size) memset( new_ptr + old_size, 0, new_size - old_size) self.size += new_size - old_size @@ -127,7 +129,7 @@ cdef class Pool: If p is not in Pool.addresses, a KeyError is raised. """ - with cython.critical_section(self): + with cython.critical_section(self, self.addresses): self.size -= self.addresses.pop(p) self.pyfree.free(p) From a7a013c1283911bd6aa5b253c8b13b89a3fd8f90 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Wed, 29 Oct 2025 18:37:26 +0100 Subject: [PATCH 06/14] Performance; only touch self.addresses twice in the happy path Co-authored-by: Guido Imperiale --- cymem/cymem.pyx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cymem/cymem.pyx b/cymem/cymem.pyx index 91c81c1..6d94a68 100644 --- a/cymem/cymem.pyx +++ b/cymem/cymem.pyx @@ -103,19 +103,20 @@ cdef class Pool: raise MemoryError("Error reallocating to %d bytes" % new_size) with cython.critical_section(self, self.addresses): - if p not in self.addresses: + try: + old_size = self.addresses.pop(p) + except KeyError: self.pyfree.free(new_ptr) raise ValueError("Pointer %d not found in Pool %s" % (p, self.addresses)) - old_size = self.addresses[p] if new_size <= old_size: self.pyfree.free(new_ptr) + self.addresses[p] = old_size raise ValueError("Realloc requires new_size > previous size") memcpy(new_ptr, p, old_size) memset( new_ptr + old_size, 0, new_size - old_size) self.size += new_size - old_size - del self.addresses[p] self.addresses[new_ptr] = new_size self.pyfree.free(p) From 52b14eaa6c34b49a5a97bc494adb140f7e0674ab Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Thu, 6 Nov 2025 15:25:58 +0100 Subject: [PATCH 07/14] Revert changes to CI for fork - remove TODOs --- .github/workflows/cibuildwheel.yml | 99 ++++++++++++++---------------- .github/workflows/tests.yml | 3 +- 2 files changed, 48 insertions(+), 54 deletions(-) diff --git a/.github/workflows/cibuildwheel.yml b/.github/workflows/cibuildwheel.yml index e3ee404..fe1da00 100644 --- a/.github/workflows/cibuildwheel.yml +++ b/.github/workflows/cibuildwheel.yml @@ -7,9 +7,6 @@ on: # ** matches 'zero or more of any character' - 'release-v[0-9]+.[0-9]+.[0-9]+**' - 'prerelease-v[0-9]+.[0-9]+.[0-9]+**' - # TODO: Remove this before upstream PR - pull_request: - types: [opened, synchronize, reopened, edited] jobs: build_wheels: name: Build wheels on ${{ matrix.os }} @@ -46,52 +43,50 @@ jobs: with: name: cibw-sdist path: dist/*.tar.gz - - # TODO: Uncomment this before upstream PR - # create_release: - # needs: [build_wheels, build_sdist] - # runs-on: ubuntu-latest - # permissions: - # contents: write - # checks: write - # actions: read - # issues: read - # packages: write - # pull-requests: read - # repository-projects: read - # statuses: read - # steps: - # - name: Get the tag name and determine if it's a prerelease - # id: get_tag_info - # run: | - # FULL_TAG=${GITHUB_REF#refs/tags/} - # if [[ $FULL_TAG == release-* ]]; then - # TAG_NAME=${FULL_TAG#release-} - # IS_PRERELEASE=false - # elif [[ $FULL_TAG == prerelease-* ]]; then - # TAG_NAME=${FULL_TAG#prerelease-} - # IS_PRERELEASE=true - # else - # echo "Tag does not match expected patterns" >&2 - # exit 1 - # fi - # echo "FULL_TAG=$TAG_NAME" >> $GITHUB_ENV - # echo "TAG_NAME=$TAG_NAME" >> $GITHUB_ENV - # echo "IS_PRERELEASE=$IS_PRERELEASE" >> $GITHUB_ENV - # - uses: actions/download-artifact@v4 - # with: - # # unpacks all CIBW artifacts into dist/ - # pattern: cibw-* - # path: dist - # merge-multiple: true - # - name: Create Draft Release - # id: create_release - # uses: softprops/action-gh-release@v2 - # if: startsWith(github.ref, 'refs/tags/') - # env: - # GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - # with: - # name: ${{ env.TAG_NAME }} - # draft: true - # prerelease: ${{ env.IS_PRERELEASE }} - # files: "./dist/*" + create_release: + needs: [build_wheels, build_sdist] + runs-on: ubuntu-latest + permissions: + contents: write + checks: write + actions: read + issues: read + packages: write + pull-requests: read + repository-projects: read + statuses: read + steps: + - name: Get the tag name and determine if it's a prerelease + id: get_tag_info + run: | + FULL_TAG=${GITHUB_REF#refs/tags/} + if [[ $FULL_TAG == release-* ]]; then + TAG_NAME=${FULL_TAG#release-} + IS_PRERELEASE=false + elif [[ $FULL_TAG == prerelease-* ]]; then + TAG_NAME=${FULL_TAG#prerelease-} + IS_PRERELEASE=true + else + echo "Tag does not match expected patterns" >&2 + exit 1 + fi + echo "FULL_TAG=$TAG_NAME" >> $GITHUB_ENV + echo "TAG_NAME=$TAG_NAME" >> $GITHUB_ENV + echo "IS_PRERELEASE=$IS_PRERELEASE" >> $GITHUB_ENV + - uses: actions/download-artifact@v4 + with: + # unpacks all CIBW artifacts into dist/ + pattern: cibw-* + path: dist + merge-multiple: true + - name: Create Draft Release + id: create_release + uses: softprops/action-gh-release@v2 + if: startsWith(github.ref, 'refs/tags/') + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + name: ${{ env.TAG_NAME }} + draft: true + prerelease: ${{ env.IS_PRERELEASE }} + files: "./dist/*" diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ed4bca5..b73968c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -19,8 +19,7 @@ env: jobs: tests: name: Test - # TODO: Uncomment this before upstream PR - # if: github.repository_owner == 'explosion' + if: github.repository_owner == 'explosion' strategy: fail-fast: false matrix: From 849b315343d04a181f14fda8130d1a833a027e5c Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Thu, 6 Nov 2025 15:38:32 +0100 Subject: [PATCH 08/14] Add comment about critical section on self and self.addresses --- cymem/cymem.pyx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cymem/cymem.pyx b/cymem/cymem.pyx index 6d94a68..a4ad362 100644 --- a/cymem/cymem.pyx +++ b/cymem/cymem.pyx @@ -82,6 +82,12 @@ cdef class Pool: if p == NULL: raise MemoryError("Error assigning %d bytes" % (number * elem_size)) memset(p, 0, number * elem_size) + + # We need a critical section on both self and self.addresses here. + # If we were just to acquire a crtitical section on self, mutating + # self.addresses would implicitly acquire a critical section on + # self.addresses, which would release our critical section on self, + # potentially allowing another thread to interleave. with cython.critical_section(self, self.addresses): self.addresses[p] = number * elem_size self.size += number * elem_size @@ -102,6 +108,8 @@ cdef class Pool: if new_ptr == NULL: raise MemoryError("Error reallocating to %d bytes" % new_size) + # We need a critical section on both self and self.addresses here. + # See comment in alloc for rationale. with cython.critical_section(self, self.addresses): try: old_size = self.addresses.pop(p) @@ -130,6 +138,8 @@ cdef class Pool: If p is not in Pool.addresses, a KeyError is raised. """ + # We need a critical section on both self and self.addresses here. + # See comment in alloc for rationale. with cython.critical_section(self, self.addresses): self.size -= self.addresses.pop(p) self.pyfree.free(p) From 37c582842e85aed12b65f8023270c0e0d05eec34 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Thu, 6 Nov 2025 15:40:22 +0100 Subject: [PATCH 09/14] Add 3.14 and 3.14t to the CI matrix --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b73968c..65c00e3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -24,7 +24,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, windows-latest, macos-latest] - python_version: ["3.9", "3.10", "3.11", "3.12", "3.13"] + python_version: ["3.9", "3.10", "3.11", "3.12", "3.13", "3.14", "3.14t"] exclude: # The 3.9/3.10 arm64 Python binaries that `setup-python` tries to use # don't work on macOS 15 anymore, so skip those. From 605ed1d28a77a3c18034d46efd5624dad4b8a5d7 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Thu, 6 Nov 2025 15:55:02 +0100 Subject: [PATCH 10/14] Fix cibuildwheel job; pure-python is optional, wheel-name-pattern is required --- .github/workflows/cibuildwheel.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cibuildwheel.yml b/.github/workflows/cibuildwheel.yml index 9524e09..94823d9 100644 --- a/.github/workflows/cibuildwheel.yml +++ b/.github/workflows/cibuildwheel.yml @@ -14,6 +14,6 @@ jobs: contents: write actions: read with: - pure-python: false + wheel-name-pattern: '*.whl' secrets: gh-token: ${{ secrets.GITHUB_TOKEN }} From 0dd50c6386f628ec083cfe7f933103770425f4c0 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Thu, 6 Nov 2025 18:46:00 +0100 Subject: [PATCH 11/14] Apply suggestions from code review Co-authored-by: Matthew Honnibal --- .github/workflows/cibuildwheel.yml | 2 +- cymem/cymem.pyx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cibuildwheel.yml b/.github/workflows/cibuildwheel.yml index 94823d9..724214d 100644 --- a/.github/workflows/cibuildwheel.yml +++ b/.github/workflows/cibuildwheel.yml @@ -14,6 +14,6 @@ jobs: contents: write actions: read with: - wheel-name-pattern: '*.whl' + wheel-name-pattern: 'cymem-*.whl' secrets: gh-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/cymem/cymem.pyx b/cymem/cymem.pyx index a4ad362..d51466a 100644 --- a/cymem/cymem.pyx +++ b/cymem/cymem.pyx @@ -117,7 +117,7 @@ cdef class Pool: self.pyfree.free(new_ptr) raise ValueError("Pointer %d not found in Pool %s" % (p, self.addresses)) - if new_size <= old_size: + if old_size >= new_size: self.pyfree.free(new_ptr) self.addresses[p] = old_size raise ValueError("Realloc requires new_size > previous size") From 53bb400ebec42e343869e29245d1bdc64e8076ce Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Fri, 7 Nov 2025 14:00:36 +0100 Subject: [PATCH 12/14] Avoid call to PyNumber_InPlaceSubtract by casting to size_t in free --- cymem/cymem.pyx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cymem/cymem.pyx b/cymem/cymem.pyx index d51466a..825ed83 100644 --- a/cymem/cymem.pyx +++ b/cymem/cymem.pyx @@ -141,7 +141,10 @@ cdef class Pool: # We need a critical section on both self and self.addresses here. # See comment in alloc for rationale. with cython.critical_section(self, self.addresses): - self.size -= self.addresses.pop(p) + # The cast to size_t below is needed so that Cython + # does not call into the C API to do the subtraction, + # which could potentially release the critical section. + self.size -= self.addresses.pop(p) self.pyfree.free(p) def own_pyref(self, object py_ref): From 05e8576bdd9bc468ac90153cd6e85feedb9db82e Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Fri, 7 Nov 2025 15:35:40 +0100 Subject: [PATCH 13/14] Acquire critical section only on self.addresses --- cymem/cymem.pyx | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/cymem/cymem.pyx b/cymem/cymem.pyx index 825ed83..2019458 100644 --- a/cymem/cymem.pyx +++ b/cymem/cymem.pyx @@ -47,6 +47,9 @@ cdef class Pool: Thread-safety: All public methods in this class can be safely called from multiple threads. + Testing the thread-safety of this class can be done by checking out the repository + at https://github.com/lysnikolaou/test-cymem-threadsafety and following the + instructions on the README. """ def __cinit__(self, PyMalloc pymalloc=Default_Malloc, @@ -83,12 +86,14 @@ cdef class Pool: raise MemoryError("Error assigning %d bytes" % (number * elem_size)) memset(p, 0, number * elem_size) - # We need a critical section on both self and self.addresses here. - # If we were just to acquire a crtitical section on self, mutating - # self.addresses would implicitly acquire a critical section on - # self.addresses, which would release our critical section on self, - # potentially allowing another thread to interleave. - with cython.critical_section(self, self.addresses): + # We need a critical section here so that addresses and size get + # updated atomically. If we were to acquire a critical section on self, + # mutating the dictionary would try to acquire a critical section on + # the dictionary and therefore release the critical section on self. + # Acquiring a critical section on self.addresses works because that's + # the only C API that gets called inside the block and acquiring the + # critical section on the top-most held lock does not release it. + with cython.critical_section(self.addresses): self.addresses[p] = number * elem_size self.size += number * elem_size return p @@ -108,9 +113,9 @@ cdef class Pool: if new_ptr == NULL: raise MemoryError("Error reallocating to %d bytes" % new_size) - # We need a critical section on both self and self.addresses here. - # See comment in alloc for rationale. - with cython.critical_section(self, self.addresses): + # See comment in alloc on why we're acquiring a critical section on + # self.addresses instead of self. + with cython.critical_section(self.addresses): try: old_size = self.addresses.pop(p) except KeyError: @@ -118,8 +123,8 @@ cdef class Pool: raise ValueError("Pointer %d not found in Pool %s" % (p, self.addresses)) if old_size >= new_size: - self.pyfree.free(new_ptr) self.addresses[p] = old_size + self.pyfree.free(new_ptr) raise ValueError("Realloc requires new_size > previous size") memcpy(new_ptr, p, old_size) @@ -138,12 +143,9 @@ cdef class Pool: If p is not in Pool.addresses, a KeyError is raised. """ - # We need a critical section on both self and self.addresses here. - # See comment in alloc for rationale. - with cython.critical_section(self, self.addresses): - # The cast to size_t below is needed so that Cython - # does not call into the C API to do the subtraction, - # which could potentially release the critical section. + # See comment in alloc on why we're acquiring a critical section on + # self.addresses instead of self. + with cython.critical_section(self.addresses): self.size -= self.addresses.pop(p) self.pyfree.free(p) From 97d1eee32a403ba9080c3a6ea9b3cc8c6af61886 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Mon, 10 Nov 2025 14:47:21 +0100 Subject: [PATCH 14/14] Avoid augassign --- cymem/cymem.pyx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cymem/cymem.pyx b/cymem/cymem.pyx index 2019458..5657d3a 100644 --- a/cymem/cymem.pyx +++ b/cymem/cymem.pyx @@ -143,10 +143,13 @@ cdef class Pool: If p is not in Pool.addresses, a KeyError is raised. """ + cdef size_t size + # See comment in alloc on why we're acquiring a critical section on # self.addresses instead of self. with cython.critical_section(self.addresses): - self.size -= self.addresses.pop(p) + size = self.addresses.pop(p) + self.size -= size self.pyfree.free(p) def own_pyref(self, object py_ref):