Repository navigation
Keep the GIL while Python callbacks can run, and raise their exceptions as they are - #766
Merged
Merged
Conversation
With blosc2.set_releasegil(True), every write path compressed without the GIL, also when a prefilter, a filler, or a codec or filter registered from Python runs during the compression. The callback then calls Python without holding the GIL and the interpreter crashes. This affects SChunk(data=), append_data(), insert_data(), update_data(), growing set_slice(), compress2(), decompress2() and vlcompress(), and also to_cframe(), which can decompress and recompress the chunks with the super-chunk's filters. Compressions now release the GIL only if none of the prefilter, codec and filters of their context is a Python one (_can_compress_without_gil()). The ids registered from Python are kept in C arrays for that, so the check needs no Python lookups, and writes next to an unrelated Python codec still release the GIL. decompress2() cannot know the codec of a chunk beforehand and keeps the GIL while any codec or filter is registered from Python, as slice reads already do. _compress_ctx() and _decompress_ctx() make the decision for all call sites; _check_cparams() and _check_dparams() use the same predicates. Co-Authored-By: Claude Opus 5.5 <[email protected]>
An exception raised in a Python prefilter, filler, postfilter, codec or filter stays set while C-Blosc2 returns an error code. The wrappers then raised their own RuntimeError or ValueError on top of it, which Python turns into "SystemError: <class 'RuntimeError'> returned a result with an exception set". Where the wrappers went on instead (the NDArray slice reads retry failed reads, a growing SChunk.set_slice() did not check its first C call, concat() released its locks first), Python code ran with the exception set. Raise the pending exception right after each such C call fails, so that e.g. a prefilter's KeyError reaches the caller as a KeyError: in _check_rc(), which most NDArray methods use and which now also checks the plain error paths of compress2(), vlcompress(), vldecompress(), vldecompress_block(), get_slice(), set_slice(), get_vlblock(), _prefilter_data() and to_cframe(), and at the error paths with cleanup of the other SChunk data and vlmeta methods, decompress(), decompress2() and the NDArray slice reads. C-Blosc2 stops at the first failing callback, which the tests check. Co-Authored-By: Claude Opus 5.5 <[email protected]>
FabianIsensee
added a commit
to FabianIsensee/python-blosc2
that referenced
this pull request
Nov 5, 2026
# Conflicts: # src/blosc2/blosc2_ext.pyx
FrancescAlted
approved these changes
Nov 6, 2026
FrancescAlted
left a comment
Member
There was a problem hiding this comment.
LGTM. And yes, please open tickets for discovered issues.
FabianIsensee
added a commit
to FabianIsensee/python-blosc2
that referenced
this pull request
Nov 6, 2026
general_postfilter, general_prefilter and general_filler hand Python
arrays of size // typesize items of the callback's dtype. When the
typesize of the buffer is smaller than the dtype's itemsize, the arrays
reach past the end of the buffer and the callback writes there: C-Blosc2
(up to now) decompresses vlmetalayers with the super-chunk's postfilter,
so reading schunk.vlmeta[...] of an SChunk with an int32 postfilter
writes 36 bytes into a 9-byte msgpack buffer, and glibc aborts later
("corrupted size vs. prev_size"). The same happens with an SChunk whose
typesize is smaller than the callback dtype. A larger typesize gives
arrays that cover part of the buffer only, so part of the result is
silently left unwritten (or zero for prefilters).
Store the itemsize with the callback and raise ValueError, before
building the arrays or writing anything, when the typesize of the buffer
differs. C-Blosc2 then stops, and the callers raise the pending
exception after the C call (_raise_callback_error(), Blosc#766).
The tests use callbacks that only record their calls, so they cannot
overrun anything even without the guard: postfilter, prefilter and
filler with typesize 1 and 8 for int32, and the vlmeta read/to_cframe()
of an SChunk with a postfilter (with the bundled C-Blosc2 the guard
refuses the vlmetalayer; with the C-Blosc2 fix the content is read
without the postfilter).
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DiPFztdpnKMJkzrjT36BuP
FabianIsensee
added a commit
to FabianIsensee/python-blosc2
that referenced
this pull request
Nov 6, 2026
general_postfilter, general_prefilter and general_filler hand Python arrays of size // typesize items of the callback's dtype. When the typesize of the buffer is smaller than the dtype's itemsize, the arrays reach past the end of the buffer and the callback writes there: C-Blosc2 before Blosc/c-blosc2#857 decompresses vlmetalayers with the super-chunk's postfilter, so reading schunk.vlmeta[...] of an SChunk with an int32 postfilter writes 36 bytes into a 9-byte msgpack buffer, and glibc aborts later ("corrupted size vs. prev_size"). The same happens with an SChunk whose typesize is smaller than the callback dtype. A larger typesize gives arrays that cover part of the buffer only, so part of the result is silently left unwritten (or zero for prefilters). Store the itemsize with the callback and raise ValueError ("postfilter: data typesize 1 does not match dtype itemsize 4"), before building the arrays or writing anything, when the typesize of the buffer differs. C-Blosc2 then stops, and the callers raise the pending exception after the C call (_raise_callback_error(), Blosc#766). SChunk.filler() now removes the filler also when an append fails: it stayed set before, with only the id of its inputs tuple, which a later append would have used after the tuple was gone. The tests use callbacks that only record their calls, so they cannot overrun anything even without the guard, and run with the GIL kept and released: postfilter, prefilter and filler with typesize 1 and 8 for int32, and the vlmeta read/to_cframe() of an SChunk with a postfilter (with the bundled C-Blosc2 the guard refuses the vlmetalayer; with Blosc/c-blosc2#857 the content is read without the postfilter). Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01DiPFztdpnKMJkzrjT36BuP
This was referenced Nov 6, 2026
Contributor
Author
|
Thanks Francesc! The issues listed in the description are all in PRs now:
Other follow-ups to this PR: #772 (crash at exit while daemon threads still run without the GIL), #774 (C prefilters that could need the GIL) and #775 (span reads of chunks with a Python codec or filter). The BLOSC_NTHREADS case mentioned in #769 (SChunks with a Python codec still run it on worker threads) is being worked on; a PR will follow. I ran into a number of smaller bugs along the way. I'll collect them in one issue once the current PRs are through, so they don't get lost. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi Francesc,
two fixes for Python callbacks: prefilters, fillers, postfilters, and codecs and filters registered from Python. Two commits on top of
main(d3bca9e), independent of my other PRs. PyPI 4.14.1 behaves the same asmain.1. Segfault with
set_releasegil(True). Withblosc2.set_releasegil(True), every write path compresses insidewith nogil:, also when a Python callback runs during the compression. The callback then creates NumPy arrays and calls Python without holding the GIL, and the interpreter crashes. This affectsSChunk(data=),append_data(),insert_data(),update_data(), a growingset_slice()(schunk[a:b] = xpast the end),compress2(),decompress2()andvlcompress()with a prefilter, a filler, or a Python codec or filter. It also affectsto_cframe()of a sparse SChunk with a postfilter, because serializing it decompresses the chunks.Fix: a compression releases the GIL only if none of the prefilter, codec and filters of its context is a Python one. The ids registered from Python are kept in two C arrays for that, so the check needs no Python lookups, and writes next to a Python codec they do not use still release the GIL.
decompress2()cannot know the codec of a chunk beforehand, so it keeps the GIL while any codec or filter is registered from Python, as the slice reads already do (_can_release_gil()). Two small wrappers,_compress_ctx()and_decompress_ctx(), make the decision for all call sites, and_check_cparams()/_check_dparams()now use the same predicates. Unlike the slice reads, these calls are not waited for inregister_codec()/register_filter(): a codec registered while such a call runs can only meet it if the call started with an id that nothing was registered under, and such a call is wrong anyway. Theset_releasegil()docstring now says when the GIL is kept.2. Exceptions turn into
SystemError. An exception raised in a callback stays set while C-Blosc2 returns an error code. The wrapper then raises its ownRuntimeErroron top of it, and the user getsSystemError: <class 'RuntimeError'> returned a result with an exception setinstead of their exception. This also happens with the GIL held. The second commit raises the pending exception right after the failing C call. Most sites now go through_check_rc(), which does that first; the error paths that free memory first call_raise_callback_error()themselves. The NDArray slice reads used to retry a failed read with the exception still set, a growingset_slice()did not check its first C call, andconcat()released its locks first; all three stop now. C-Blosc2 stops at the first failing callback, so no Python code runs with the exception set. The tests check that each failing callback runs exactly once.Performance: measured against
mainwith interleaved runs (4 KiB and 256 KiB chunks, 1 and 4 threads,set_releasegilon and off, a Python codec registered or not): all cases are within ±2% ofmain, including multi-threaded writes next to a registered Python codec. The one exception is by design:decompress2()from several threads while a Python codec or filter is registered keeps the GIL, as slice reads do, and is ~3x slower in that case than onmain(where it would crash if a chunk used that codec).Tests: new
tests/test_releasegil_callbacks.py, 113 cases:main, all 16 GIL-released cases segfault (each run in its own process); the GIL-held ones pass.KeyErroron writes and on reads, for SChunks and NDArrays: theKeyErrormust reach the caller, the callback must have run once, and the data must be unchanged.compress2(),vlcompress(),decompress2()anddecompress()with a Python codec or filter must raise the callback's ownRuntimeError("Cannot apply user codec without an SChunk"). Without the second commit, 80 of the 113 cases fail withSystemError.test_write_releases_the_gil_next_to_python_codecschecks, liketest_get_slice_releases_the_gil, that a write keeps running without the GIL while an unrelated Python codec and filter are registered.pytest tests: 10747 passed. The 2 failures are theb2viewhttp tests that need aiohttp, as onmain.Not fixed here, found on the way (I can open issues):
decompress2()of a chunk made with a Python codec or filter segfaults with more than one thread (its default), with or without the GIL, and so doesdecompress()afterset_nthreads(n > 1): the decoder runs on C-Blosc2's worker threads. SChunks refuse that in_check_dparams(), but these functions cannot see the chunk's codec.blosc2_vlmeta_get()decompresses the vlmetalayers with the SChunk's dparams, postfilter included. A Python postfilter then runs on the vlmeta bytes as if they were items of its dtype, which overruns the buffer (heap corruption, also when just readingschunk.vlmeta[...]).to_cframe()of an SChunk with a postfilter stores the postfiltered data for every storage except an in-memory contiguous one (schunk_copy_skeleton()forces the filters when copying).vlmetaread leaks the decompressed content (get_vlmeta()never frees it).AI disclosure: the analysis, fix and tests were done with Claude Code; I reviewed them.
Best,
Fabian
🤖 Generated with Claude Code