From 8ebe69f9ddde24b20d81f2983c4239b18bde0fc3 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 19 Jun 2024 13:31:08 -0400 Subject: [PATCH 1/8] Update supported versions. --- .github/workflows/test.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4a51f14..9c30658 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -21,14 +21,15 @@ jobs: - "ubuntu-22.04" - "windows-2022" python-version: - - "3.7" - "3.8" - "3.9" - "3.10" - "3.11" - - "pypy-3.7" + - "3.12" + - "3.13-dev" - "pypy-3.8" - "pypy-3.9" + - "pypy-3.10" steps: From 063cdd7f804f34f1a688984b63a739eeb3615a3a Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 19 Jun 2024 13:34:53 -0400 Subject: [PATCH 2/8] Newer macOS. --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9c30658..852ee25 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,7 +17,7 @@ jobs: # Python versions in sync with the ones in build.yml. matrix: os: - - "macos-11" + - "macos-latest" - "ubuntu-22.04" - "windows-2022" python-version: From 91d9a05d48870158171fadc156a93ad83d67163e Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 19 Jun 2024 13:35:00 -0400 Subject: [PATCH 3/8] Match test.yml. --- .github/workflows/build.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c3d3f5b..f7e7acc 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -35,7 +35,7 @@ jobs: # Python versions in sync with the ones in test.yml. matrix: os: - - "macos-11" + - "macos-12" - "ubuntu-22.04" - "windows-2022" wheel-selector: @@ -44,9 +44,10 @@ jobs: - "cp310-*" - "cp311-*" - "cp312-*" - - "pp37-*" + - "cp313-*" - "pp38-*" - "pp39-*" + - "pp310-*" steps: - name: Check out zfec sources From e745e26f72c8caf8fcfe28c5105222b05da819b7 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 20 Jun 2024 14:01:52 -0400 Subject: [PATCH 4/8] Start switch to modern buffer API. --- zfec/_fecmodule.c | 69 +++++++++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/zfec/_fecmodule.c b/zfec/_fecmodule.c index 846101f..6b6f612 100644 --- a/zfec/_fecmodule.c +++ b/zfec/_fecmodule.c @@ -123,6 +123,7 @@ Encoder_encode(Encoder *self, PyObject *args) { PyObject** pystrs_produced = (PyObject**)alloca((self->mm - self->kk) * sizeof(PyObject*)); /* This is an upper bound -- we will actually use only num_check_blocks_produced of these elements (see below). */ unsigned num_check_blocks_produced = 0; /* The first num_check_blocks_produced elements of the check_blocks_produced array and of the pystrs_produced array will be used. */ const gf** incblocks = (const gf**)alloca(self->kk * sizeof(const gf*)); + Py_buffer *pybufs = (Py_buffer *)alloca(self->kk * sizeof(Py_buffer)); size_t num_desired_blocks; PyObject* fast_desired_blocks_nums = NULL; PyObject** fast_desired_blocks_nums_items; @@ -131,7 +132,7 @@ Encoder_encode(Encoder *self, PyObject *args) { size_t i; PyObject* fastinblocks = NULL; PyObject** fastinblocksitems; - Py_ssize_t sz, oldsz = 0; + Py_ssize_t oldsz = 0; unsigned char check_block_index = 0; /* index into the check_blocks_produced and (parallel) pystrs_produced arrays */ if (!PyArg_ParseTuple(args, "O|O:Encoder.encode", &inblocks, &desired_blocks_nums)) @@ -179,18 +180,23 @@ Encoder_encode(Encoder *self, PyObject *args) { if (!fastinblocksitems) goto err; + // TODO on an error, we leak references. for (i = 0; i < self->kk; i++) { - if (!PyObject_CheckReadBuffer(fastinblocksitems[i])) { - PyErr_Format(py_fec_error, "Precondition violation: %zu'th item is required to offer the single-segment read character buffer protocol, but it does not.", i); - goto err; - } - if (PyObject_AsReadBuffer(fastinblocksitems[i], (const void**)&(incblocks[i]), &sz)) - goto err; - if (oldsz != 0 && oldsz != sz) { - PyErr_Format(py_fec_error, "Precondition violation: Input blocks are required to be all the same length. length of one block was: %zu, length of another block was: %zu", oldsz, sz); - goto err; - } - oldsz = sz; + if (PyObject_GetBuffer(fastinblocksitems[i], &pybufs[i], 0)) + goto err; + if (!PyBuffer_IsContiguous(&pybufs[i], 'C')) { + goto err; + } + if (oldsz != 0 && oldsz != pybufs[i].len) { + PyErr_Format(py_fec_error, + "Precondition violation: Input blocks are required to be " + "all the same length. length of one block was: %zu, " + "length of another block was: %zu", + oldsz, pybufs[i].len); + goto err; + } + incblocks[i] = pybufs[i].buf; + oldsz = pybufs[i].len; } /* Allocate space for all of the check blocks. */ @@ -198,7 +204,7 @@ Encoder_encode(Encoder *self, PyObject *args) { for (i = 0; i < num_desired_blocks; i++) { if (c_desired_blocks_nums[i] >= self->kk) { c_desired_checkblocks_ids[check_block_index] = c_desired_blocks_nums[i]; - pystrs_produced[check_block_index] = PyString_FromStringAndSize(NULL, sz); + pystrs_produced[check_block_index] = PyString_FromStringAndSize(NULL, oldsz); if (pystrs_produced[check_block_index] == NULL) goto err; check_blocks_produced[check_block_index] = (gf*)PyString_AsString(pystrs_produced[check_block_index]); @@ -211,7 +217,7 @@ Encoder_encode(Encoder *self, PyObject *args) { /* Encode any check blocks that are needed. */ Py_BEGIN_ALLOW_THREADS - fec_encode(self->fec_matrix, incblocks, check_blocks_produced, c_desired_checkblocks_ids, num_check_blocks_produced, sz); + fec_encode(self->fec_matrix, incblocks, check_blocks_produced, c_desired_checkblocks_ids, num_check_blocks_produced, oldsz); Py_END_ALLOW_THREADS /* Wrap all requested blocks up into a Python list of Python strings. */ @@ -240,10 +246,15 @@ Encoder_encode(Encoder *self, PyObject *args) { Py_XDECREF(pystrs_produced[i]); Py_XDECREF(result); result = NULL; cleanup: - Py_XDECREF(fastinblocks); fastinblocks=NULL; - Py_XDECREF(fast_desired_blocks_nums); fast_desired_blocks_nums=NULL; - return result; -} + for (i = 0; i < self->kk; i++) { + // TODO release Py_buffers + } + Py_XDECREF(fastinblocks); + fastinblocks = NULL; + Py_XDECREF(fast_desired_blocks_nums); + fast_desired_blocks_nums = NULL; + return result; + } static void Encoder_dealloc(Encoder * self) { @@ -390,6 +401,7 @@ Decoder_decode(Decoder *self, PyObject *args) { PyObject* result = NULL; const gf**restrict cblocks = (const gf**restrict)alloca(self->kk * sizeof(const gf*)); + Py_buffer *pybufs = (Py_buffer *)alloca(self->kk * sizeof(Py_buffer)); unsigned* cblocknums = (unsigned*)alloca(self->kk * sizeof(unsigned)); gf**restrict recoveredcstrs = (gf**)alloca(self->kk * sizeof(gf*)); /* self->kk is actually an upper bound -- we probably won't need all of this space. */ PyObject**restrict recoveredpystrs = (PyObject**restrict)alloca(self->kk * sizeof(PyObject*)); /* self->kk is actually an upper bound -- we probably won't need all of this space. */ @@ -399,7 +411,7 @@ Decoder_decode(Decoder *self, PyObject *args) { unsigned needtorecover=0; PyObject** fastblocksitems; PyObject** fastblocknumsitems; - Py_ssize_t sz, oldsz = 0; + Py_ssize_t oldsz = 0; long tmpl; unsigned nextrecoveredix=0; @@ -446,17 +458,15 @@ Decoder_decode(Decoder *self, PyObject *args) { if (cblocknums[i] >= self->kk) needtorecover+=1; - if (!PyObject_CheckReadBuffer(fastblocksitems[i])) { - PyErr_Format(py_fec_error, "Precondition violation: %u'th item is required to offer the single-segment read character buffer protocol, but it does not.\n", i); - goto err; - } - if (PyObject_AsReadBuffer(fastblocksitems[i], (const void**)&(cblocks[i]), &sz)) + // TODO object reference leaks on error? + if (PyObject_GetBuffer(fastblocksitems[i], &pybufs[i], 0)) goto err; - if (oldsz != 0 && oldsz != sz) { - PyErr_Format(py_fec_error, "Precondition violation: Input blocks are required to be all the same length. length of one block was: %zu, length of another block was: %zu\n", oldsz, sz); + if (oldsz != 0 && oldsz != pybufs[i].len) { + PyErr_Format(py_fec_error, "Precondition violation: Input blocks are required to be all the same length. length of one block was: %zu, length of another block was: %zu\n", oldsz, pybufs[i].len); goto err; } - oldsz = sz; + cblocks[i] = pybufs[i].buf; + oldsz = pybufs[i].len; } /* Move src packets into position. At the end of this loop we want the i'th @@ -477,7 +487,7 @@ Decoder_decode(Decoder *self, PyObject *args) { /* Allocate space for all of the recovered blocks. */ for (i=0; ifec_matrix, cblocks, recoveredcstrs, cblocknums, sz); + fec_decode(self->fec_matrix, cblocks, recoveredcstrs, cblocknums, oldsz); Py_END_ALLOW_THREADS /* Wrap up both original primary blocks and decoded blocks into a Python list of Python strings. */ @@ -517,6 +527,7 @@ Decoder_decode(Decoder *self, PyObject *args) { Py_XDECREF(recoveredpystrs[i]); Py_XDECREF(result); result = NULL; cleanup: + // TODO clean up Py_buffers Py_XDECREF(fastblocks); fastblocks=NULL; Py_XDECREF(fastblocknums); fastblocknums=NULL; return result; From 338724a2d1da75fd7f8c0cc6e868659fbd4841fe Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 20 Jun 2024 14:24:19 -0400 Subject: [PATCH 5/8] Clean up buffers on errors or return. --- zfec/_fecmodule.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/zfec/_fecmodule.c b/zfec/_fecmodule.c index 6b6f612..7156d67 100644 --- a/zfec/_fecmodule.c +++ b/zfec/_fecmodule.c @@ -124,6 +124,9 @@ Encoder_encode(Encoder *self, PyObject *args) { unsigned num_check_blocks_produced = 0; /* The first num_check_blocks_produced elements of the check_blocks_produced array and of the pystrs_produced array will be used. */ const gf** incblocks = (const gf**)alloca(self->kk * sizeof(const gf*)); Py_buffer *pybufs = (Py_buffer *)alloca(self->kk * sizeof(Py_buffer)); + for (int i = 0; i < self->kk; i++) { + pybufs[i].buf = NULL; + } size_t num_desired_blocks; PyObject* fast_desired_blocks_nums = NULL; PyObject** fast_desired_blocks_nums_items; @@ -180,7 +183,6 @@ Encoder_encode(Encoder *self, PyObject *args) { if (!fastinblocksitems) goto err; - // TODO on an error, we leak references. for (i = 0; i < self->kk; i++) { if (PyObject_GetBuffer(fastinblocksitems[i], &pybufs[i], 0)) goto err; @@ -246,15 +248,17 @@ Encoder_encode(Encoder *self, PyObject *args) { Py_XDECREF(pystrs_produced[i]); Py_XDECREF(result); result = NULL; cleanup: - for (i = 0; i < self->kk; i++) { - // TODO release Py_buffers - } - Py_XDECREF(fastinblocks); - fastinblocks = NULL; - Py_XDECREF(fast_desired_blocks_nums); - fast_desired_blocks_nums = NULL; - return result; + for (i = 0; i < self->kk; i++) { + if (pybufs[i].buf != NULL) { + PyBuffer_Release(&pybufs[i]); + } } + Py_XDECREF(fastinblocks); + fastinblocks = NULL; + Py_XDECREF(fast_desired_blocks_nums); + fast_desired_blocks_nums = NULL; + return result; +} static void Encoder_dealloc(Encoder * self) { @@ -402,6 +406,9 @@ Decoder_decode(Decoder *self, PyObject *args) { const gf**restrict cblocks = (const gf**restrict)alloca(self->kk * sizeof(const gf*)); Py_buffer *pybufs = (Py_buffer *)alloca(self->kk * sizeof(Py_buffer)); + for (int i = 0; i < self->kk; i++) { + pybufs[i].buf = NULL; + } unsigned* cblocknums = (unsigned*)alloca(self->kk * sizeof(unsigned)); gf**restrict recoveredcstrs = (gf**)alloca(self->kk * sizeof(gf*)); /* self->kk is actually an upper bound -- we probably won't need all of this space. */ PyObject**restrict recoveredpystrs = (PyObject**restrict)alloca(self->kk * sizeof(PyObject*)); /* self->kk is actually an upper bound -- we probably won't need all of this space. */ @@ -458,9 +465,10 @@ Decoder_decode(Decoder *self, PyObject *args) { if (cblocknums[i] >= self->kk) needtorecover+=1; - // TODO object reference leaks on error? - if (PyObject_GetBuffer(fastblocksitems[i], &pybufs[i], 0)) + if (PyObject_GetBuffer(fastblocksitems[i], &pybufs[i], 0)) { + pybufs[i].buf = NULL; goto err; + } if (oldsz != 0 && oldsz != pybufs[i].len) { PyErr_Format(py_fec_error, "Precondition violation: Input blocks are required to be all the same length. length of one block was: %zu, length of another block was: %zu\n", oldsz, pybufs[i].len); goto err; @@ -527,7 +535,11 @@ Decoder_decode(Decoder *self, PyObject *args) { Py_XDECREF(recoveredpystrs[i]); Py_XDECREF(result); result = NULL; cleanup: - // TODO clean up Py_buffers + for (i = 0; i < self->kk; i++) { + if (pybufs[i].buf != NULL) { + PyBuffer_Release(&pybufs[i]); + } + } Py_XDECREF(fastblocks); fastblocks=NULL; Py_XDECREF(fastblocknums); fastblocknums=NULL; return result; From c3f2f229ff5dd1d09404583d3342f2e53867d809 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 20 Jun 2024 14:31:14 -0400 Subject: [PATCH 6/8] Add support for prereleases, update to latest cibuildwheel --- .github/workflows/build.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f7e7acc..e1109a6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,6 +23,8 @@ on: release: types: [published, created, edited] +env: + CIBW_PRERELEASE_PYTHONS: "True" jobs: build_wheels: @@ -58,7 +60,7 @@ jobs: fetch-depth: 0 - name: Build wheels - uses: pypa/cibuildwheel@v2.16.2 + uses: pypa/cibuildwheel@v2.19.1 with: output-dir: wheelhouse env: From 843a48ea041c690efdc4eb270deb580054c7e660 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 20 Jun 2024 14:32:51 -0400 Subject: [PATCH 7/8] More to correct location --- .github/workflows/build.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e1109a6..659d591 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,9 +23,6 @@ on: release: types: [published, created, edited] -env: - CIBW_PRERELEASE_PYTHONS: "True" - jobs: build_wheels: name: "${{ matrix.wheel-selector }} wheel on ${{ matrix.os }}" @@ -74,7 +71,8 @@ jobs: CIBW_ARCHS_MACOS: "x86_64 universal2 arm64" # Skip testing arm64 builds on Intel Macs. CIBW_TEST_SKIP: "*-macosx_arm64 *-macosx_universal2:arm64" - + # Allow beta releases of Python: + CIBW_PRERELEASE_PYTHONS: "True" - uses: actions/upload-artifact@v3 name: Upload artifacts with: From 820e0e6cf2b2eba8fc260bef6a7a36050595061a Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 5 Aug 2024 16:56:05 -0400 Subject: [PATCH 8/8] Switch to 3.13 rc building --- .github/workflows/build.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 659d591..58f3f88 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -57,7 +57,7 @@ jobs: fetch-depth: 0 - name: Build wheels - uses: pypa/cibuildwheel@v2.19.1 + uses: pypa/cibuildwheel@v2.20.0 with: output-dir: wheelhouse env: @@ -71,8 +71,6 @@ jobs: CIBW_ARCHS_MACOS: "x86_64 universal2 arm64" # Skip testing arm64 builds on Intel Macs. CIBW_TEST_SKIP: "*-macosx_arm64 *-macosx_universal2:arm64" - # Allow beta releases of Python: - CIBW_PRERELEASE_PYTHONS: "True" - uses: actions/upload-artifact@v3 name: Upload artifacts with: