Skip to content

Free b2nd contexts, arrays and buffers on error paths - #770

Open
FabianIsensee wants to merge 1 commit into
Blosc:mainfrom
FabianIsensee:fix-b2nd-ctx-leaks
Open

FabianIsensee wants to merge 1 commit into
Blosc:mainfrom
FabianIsensee:fix-b2nd-ctx-leaks

Conversation

@FabianIsensee

Copy link
Copy Markdown
Contributor

concat() and the other functions in blosc2_ext.pyx that build a b2nd context (uninit, nans, empty, zeros, full, from_buffer, asarray, NDArray.copy, NDArray.get_slice) call _check_rc() on the C call before b2nd_free_ctx(), so every failing call leaked its context (about 1 kB). Example: blosc2.concat([a, b], urlpath="no_such_dir/x.b2nd") raises RuntimeError and leaves 1088 bytes behind per call; the same happens for blosc2.empty(..., urlpath=<unwritable>), arr.copy(urlpath=...), or blosc2.full((100,), "abc", dtype=np.int32) (the fill value fails after the context exists).

Looking at the other functions that create C resources turned up more:

  • create_b2nd_context() malloc'd a blosc2_cparams and a blosc2_dparams for every array, and the names and contents of its meta= metalayers, and never freed them: 192 bytes per array created (plus the metalayers), also on success. b2nd_create_ctx() copies the cparams but only points to the dparams and the metalayer buffers, and b2nd_free_ctx() frees only its own copies.
  • A buffer export of the input stayed alive when the call raised, in asarray() (the user's whole array was never freed), a[0:10, 0:20] = <wrong size> (NDArray.set_slice()), a.oindex[...] = ... / a.oindex[...] with an index out of bounds, schunk_from_cframe() / ndarray_from_cframe() on invalid input, SChunk.decompress_chunk(n, dst=<empty>), SChunk.get_slice(out=<too small>) and compress2() when the context cannot be created (e.g. an unknown filter id). A bytearray passed there can no longer be resized afterwards (BufferError: Existing exports of data).
  • blosc2.open() of a frame whose b2nd metalayer cannot be decoded ("Could not create array from schunk", with or without mmap_mode), or when check_schunk_params() raises (blosc2.open(path, cparams={"typesize": 2}, typesize=2)), kept the super-chunk and its open file: one file descriptor per call.
  • NDArray.get_slice() kept the sliced array when b2nd_squeeze_index() failed (e.g. a.slice(key, meta={...})), get/set_oindex_numpy() kept their index arrays.

What this PR does (blosc2_ext.pyx only):

  • create_b2nd_context() returns a small _B2ndContext object that owns the context, the dparams and the metalayer buffers it points to and frees them in __dealloc__, so all of it goes away on success and on every error path; callers use ctx.ptr and no longer call b2nd_free_ctx() themselves (the "Error while freeing the context" checks are gone, b2nd_free_ctx() cannot fail). The cparams are a local initialised from BLOSC2_CPARAMS_DEFAULTS (b2nd_create_ctx() copies them).
  • The metalayer table keeps its size (B2ND_MAX_METALAYERS, 15). More than 15 meta= entries used to overflow it on the stack (the process aborted with "stack smashing detected"); they now raise ValueError.
  • The functions above release their buffers before raising (full() and asarray() too), get/set_oindex_numpy() free their index arrays in a finally.
  • open() frees the array (and with it the super-chunk) or the super-chunk if it raises before a Python object owns them.
  • NDArray.get_slice() frees the sliced array when the squeeze fails.

Exceptions and messages are unchanged.

Tests (tests/test_error_path_leaks.py, all 20 fail on main, one of them by aborting the process, and pass with the PR):

  • C memory: malloc's in-use bytes (glibc mallinfo2(), skipped elsewhere) per failing concat/empty/full/copy/slice(meta=...)/oindex call and per successful empty() with and without meta=; smallest growth over 3 rounds of 300 calls after a warm-up, thresholds 256 or 64 bytes (main leaks 1088, 109000, 175, 192 and 320 bytes per call; the PR 0). Python objects do not show up in this counter (pymalloc arenas are mmap'ed directly), so this is not an RSS threshold; repeated runs, with and without threads and under xdist, never came close.
  • Buffers: a weakref to the input dies after the failing call (asarray, setitem, oindex setitem), or the bytearray can be resized again (*_from_cframe, compress2, SChunk dst/out). The compress2 one is skipped on Windows: its trigger (an unknown filter id) corrupts the heap in c-blosc2 there, see below.
  • meta= with 16 entries raises ValueError, 15 work.
  • Files: failing blosc2.open() calls (bad b2nd metalayer with and without mmap, check_schunk_params() for an NDArray and a SChunk) do not keep file descriptors (/proc/self/fd or /dev/fd, skipped on WASM).

Full suite: same results as main. The touched areas (about 900 tests) and the repro scripts also ran under AddressSanitizer without reports.

Perf: the success paths do the same calls as before minus the two mallocs per array. Measured on a shared Linux node, pinned to one NUMA node (interleaved, 5 rounds each of main, a second build of main, and this PR; µs per call): concat of two 100x20 int32 arrays 24.6 / 24.1 / 22.7, empty((100,)) 29.2 / 28.4 / 26.7, asarray of 1000 int32 45.0 / 42.0 / 39.7, i.e. array creation slightly faster (a second run gave the same). a[10:20, :] = x and oindex get/set: no measurable change (the two builds of main differ by up to 4% there, single rounds by 10-15%).

Not in this PR:

  • NDArray.slice() through get_slice() leaks about 106 kB per call on success: b2nd_squeeze_index() replaces the sliced array by a view of its data and the array itself is never freed (it cannot simply be freed, the view uses its chunks). Keeping it as the view's base would fix it but changes what .base returns, so I would rather ask first. Relatedly, a.slice(key, meta=...) always fails in the squeeze (non-b2nd metalayer).
  • In c-blosc2, b2nd_from_cbuffer() (e.g. blosc2.frombuffer(b"x" * 10, (100,), dtype=np.int32), 74 kB per call) and b2nd_concatenate() leak the array they created when a later step fails; the bindings cannot free it since they do not know whether it was set.
  • In c-blosc2, blosc2_create_cctx() frees its context with free() on the "filter (%d) is not yet defined" paths although it was allocated with my_malloc() (_aligned_malloc() on Windows), so e.g. blosc2.compress2(b"x" * 1000, filters=[10]) corrupts the heap and kills the process on Windows. That needs a small c-blosc2 fix (my_free()).

I used Claude Code (Anthropic) to find and measure the leaks and to write the fix and the tests.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DiPFztdpnKMJkzrjT36BuP

concat() and the other functions that build a b2nd context (uninit,
nans, empty, zeros, full, from_buffer, asarray, NDArray.copy and
NDArray.get_slice) raised before b2nd_free_ctx() when the C call
failed, leaking the context (about 1 kB per call).

create_b2nd_context() also malloc'd the cparams and dparams of every
new array and the names and contents of its metalayers, and never
freed them (192 bytes per array plus the metalayers, also on success).
b2nd_create_ctx() copies the cparams, so they are a local now; the
context keeps pointing to the dparams and the metalayers.

create_b2nd_context() now returns a small _B2ndContext object that
owns the context, the dparams and the metalayer buffers it points to
and frees them when it goes away, on success and on every error path,
so the callers no longer free the context themselves.  Its metalayer
table has B2ND_MAX_METALAYERS entries, like the stack table before;
more metalayers now raise ValueError instead of overflowing it (the
process aborted with "stack smashing detected").

Other error paths that kept a resource:
- asarray(), full(), compress2(), NDArray.set_slice(),
  NDArray.get/set_oindex_numpy(), schunk_from_cframe(),
  ndarray_from_cframe(), SChunk.decompress_chunk(dst=...) and
  SChunk.get_slice(out=...) kept a buffer export of their input, which
  was then never freed (a bytearray could not be resized any more)
- open() kept the super-chunk and its open file when the b2nd
  metalayer could not be decoded or check_schunk_params() raised
- NDArray.get_slice() kept the sliced array when b2nd_squeeze_index()
  failed
- NDArray.get/set_oindex_numpy() kept the index arrays

The exceptions and their messages do not change (the "Error while
freeing the context" checks are gone: b2nd_free_ctx() cannot fail).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DiPFztdpnKMJkzrjT36BuP

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant