fix: output buffer growth, callback GIL use, concurrent-use and clone safety - #15
Merged
Merged
Conversation
Same fix as upstream tuxu#31. Output size: a src_process() call can generate more than ceil(input_frames * ratio) + 10000. With end_of_input the converter flushes the input it held back (about ratio x its filter half-length: 144 input frames for sinc_best, so ~37k output frames at ratio 256), and after a ratio change libsamplerate ramps from the previous ratio, which has no fixed bound. Both raised "Generated more output samples than expected!". Resampler.process() now keeps calling src_process() with a larger buffer until libsamplerate stops short of filling it; resample() uses the same path (src_simple() cannot continue). Callback: hold the GIL for the whole of the_callback_func, since the buffer_info destructor releases a Python buffer, and store any exception (ours or one raised by the Python callback) as an exception_ptr instead of a message string, so the original type is re-raised by read() and nothing unwinds through libsamplerate's C frames. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
shauneccles
changed the base branch from
ci/harden-and-upstream-sync
to
main
September 27, 2026 04:10
…lone Found reviewing the GIL handling; each reproduced before the fix: - Releasing the GIL let two Python threads drive one SRC_STATE at once: four threads calling process() on one Resampler got "Internal error: Bad length in prepare_data ()", and __exit__ from another thread during read() segfaulted. Every method that touches the state now takes a per-object in-use flag and raises RuntimeError if it's already set. A mutex could deadlock, since the holder needs the GIL to grow the output buffer while a waiter may hold it. - src_clone() copies the callback data pointer, so a clone called back into the original CallbackResampler and segfaulted once the original was freed. read() now publishes the active resampler in a thread_local, which the callback uses instead. - A clone's libsamplerate state still points into the original's last callback buffer; the copy now holds a reference to it, where it used to read freed memory. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
shauneccles
changed the title
It failed on a shared macos-15-intel runner (0.88x) while passing on the same code elsewhere; the other speedup checks in this file already warn. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Ports the fixes from upstream tuxu#31 (rebased and updated in response to review).
Output size. The fixed
+10000allowance raisesGenerated more output samples than expected!in two ordinary cases:end_of_inputflushes about ratio × the converter's filter half-length (144 input frames for sinc_best), so it fails with sinc_best above ratio ~70.process(x, 4.0)followed byprocess(200_000 frames, 0.25)fails for every converter.Resampler.process()now keeps callingsrc_process()with a bigger buffer until libsamplerate stops short of filling it, andresample()goes through the same path. The headroom constant (now 1024) only affects how often the buffer is regrown.Callback.
the_callback_funcnow holds the GIL for its whole body:buffer_info's destructor released a Python buffer after the GIL block ended. Errors are stored as anexception_ptrrather than a string, so a Python callback's own exception comes back with its original type, and nothing unwinds through libsamplerate's C code (which MSVC doesn't guarantee to support).Concurrency and clone safety (second commit, from a review of the GIL handling; each one reproduced first):
Resamplercorrupted libsamplerate state (Bad length in prepare_data ()), and__exit__from another thread duringread()segfaulted. Methods now take a per-object in-use flag and raiseRuntimeErroron concurrent or re-entrant use.CallbackResampler.clone(): the clone called back into the original (viasrc_clone), and segfaulted once the original was freed. It also read the original's freed input buffer. The callback now finds its resampler through athread_localset byread(), and the clone keeps a reference to the buffer.Tests. Seven new tests: the high-ratio flush (3 converters), the ratio change, a callback that raises, a clone that outlives its original, and re-entrant use. All 213 pass locally on Python 3.14 with NumPy 2.5.3.
🤖 Generated with Claude Code