Repository navigation
Free the content of vlmeta reads; fix metalayer reads of missing names - #771
Open
FabianIsensee wants to merge 1 commit into
Open
FabianIsensee wants to merge 1 commit into
FabianIsensee wants to merge 1 commit into
Conversation
vlmeta.get_vlmeta() copied the buffer that blosc2_vlmeta_get() mallocs into a bytes object and never freed it, so every vlmeta read (including `name in vlmeta`, `vlmeta[:]` and NDArray attribute reads) leaked the decompressed content. vlmeta.get_names() and to_dict() leaked their array of name pointers in the same way. to_dict() also kept those pointers across its get_vlmeta() calls, which re-sync (free and reload) the vlmetalayers when another handle has rewritten the frame, so it read freed names. It now copies the names (as the bytes keys it returns) and frees the array before reading. meta__getitem__() ignored the return code of blosc2_meta_get(): for a missing name it copied and freed an uninitialized pointer, so `schunk.meta["missing"] = x` (which does not check the name first) aborted the process. It now raises KeyError, or RuntimeError for other errors, and meta__setitem__() passes the name as given so the message names it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DiPFztdpnKMJkzrjT36BuP
This was referenced Nov 6, 2026
This branch has not been deployed
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.
Every vlmeta read leaked its decompressed content.
vlmeta.get_vlmeta()copies the buffer thatblosc2_vlmeta_get()mallocs into a bytes object (return content[:content_len]) and never frees it, sos.vlmeta[name],name in s.vlmeta,s.vlmeta[:],repr(s.vlmeta)and NDArray attribute reads all leak the size of the value. Trigger: 5000 reads of a 100 kB vlmeta take the peak RSS from 64 to 541 MiB on main (5ff9823); with this PR it stays at 55 MiB.Auditing
blosc2_ext.pyxfor the same pattern (C-allocated buffers turned into Python objects) found three more problems in the same functions:vlmeta.get_names()(iterating a vlmeta) andvlmeta.to_dict()leak their array of name pointers: 8 bytes per vlmetalayer per call.to_dict()kept those pointers across itsget_vlmeta()calls. The first one re-syncs the vlmetalayers (frame_check_stale()frees and reloads them) when another handle has rewritten the frame, so the remaining names pointed to freed memory: with one handle reading and another one updating a vlmeta,to_dict()on the reader raisedKeyErrorfor garbage names.meta__getitem__()did not check the return code ofblosc2_meta_get(): for a missing name it copied and freed an uninitialized pointer.Meta.__getitem__checks the name first,Meta.__setitem__does not, soschunk.meta["missing"] = 1aborted the process ("double free or corruption").The other functions that return C buffers do not leak (measured with glibc's
mallinfo2()per call:get_chunk(),get_lazychunk(),decompress_chunk(),get_vlblock(),vldecompress(),vldecompress_block(),_special_chunk(),to_cframe(),get_slice_nchunks(),get_1d_span_numpy(), and the success path of metalayer reads). The graph/artifact code from #741 does not add this pattern: its JSON and name strings are borrowed from handles that the extension types free, and its chunk caches are freed.What this PR changes (
blosc2_ext.pyxonly):get_vlmeta()frees the content after copying it. On errorsblosc2_vlmeta_get()has already freed it; for an empty value it is NULL.get_names()andto_dict()free the name array;to_dict()copies the names (as the bytes keys it returns) before it reads any value.meta__getitem__()raisesKeyErrorfor a missing name (with the messageMeta.__getitem__uses) andRuntimeErrorfor other errors, and frees the content.meta__setitem__()passes the name on as given, so theKeyErrornames it.schunk.meta["missing"] = 1now raisesKeyError: 'missing not found'.Tests (all fail without the change):
tests/test_vlmeta.py::test_vlmeta_read_frees_content: 200 reads of a 1 MiB vlmeta must not grow the RSS by 50 MiB (main: +200 MiB; this PR: at most 0.4 MiB over 30 runs).tests/test_vlmeta.py::test_vlmeta_names_are_freed: 1000 listings of 4096 names must not grow the RSS by 8 MiB (main: +31 MiB; this PR: 0).tests/test_swmr.py::test_vlmeta_to_dict_follows:to_dict()on a reader after another handle updated a vlmeta.tests/test_schunk.py::test_schunk_meta_setitem_missing_name.The two RSS tests are Linux-only and follow
test_failed_data_write_without_copy_frees_chunkintests/test_schunk_update.py; their bounds are 4x below the leak and far above the noise. With LeakSanitizer (LD_PRELOAD=libasan.so,detect_leaks=1) the vlmeta contents and the two name arrays are the only leak reports that go away; everything else it reports is the same before and after (import-time allocations). Full suite on main 5ff9823: same results as main plus the 4 new tests.Perf (measured before the rebase onto #741, on d3bca9e; the changed functions are the same; 1 thread, per call, median of 5 interleaved rounds, main built twice for the noise floor; reproduced in three runs): a 100 kB vlmeta read takes 11.1 µs instead of 63.1 µs in memory and 17.5 µs instead of 68.7 µs from a file, since the leaking version faulted in fresh pages for every read. Small reads and iterating 8 names are 2-4% faster (noise floor within 1%); small file-backed reads and metalayer reads are unchanged (within 1%).
Based on main 5ff9823; it does not depend on other open PRs.
I used Claude Code (Anthropic) to audit the code, write the fix and the tests, and measure them; I reviewed the changes and the numbers.
🤖 Generated with Claude Code
https://claude.ai/code/session_01DiPFztdpnKMJkzrjT36BuP