Skip to content

Fix bogus ~5.7 GB allocation when collecting decoded samples - #341

Closed
SepiRosho wants to merge 0 commit into
marin-m:mainfrom
SepiRosho:main
Closed

SepiRosho wants to merge 0 commit into
marin-m:mainfrom
SepiRosho:main

Conversation

@SepiRosho

Copy link
Copy Markdown
Contributor

Rodio 0.22.2's WAV decoder increments its read counter even when polled past EOF, which the resampler does at the end of the stream, so its u32 size_hint() underflows to ~2^32 (~2^32/3 after 48 kHz -> 16 kHz). Vec::collect() reserves from that hint whenever it runs out of capacity, so inputs whose sample count makes the Vec reallocate at that moment abort with "memory allocation of ~5727254732 bytes failed". Opus and other formats are affected too since they go through FFmpeg -> WAV.

Collect the samples with plain pushes so that only the Vec's own amortized growth is used, and add a regression test covering the affected lengths.

@marin-m

marin-m commented Sep 24, 2026

Copy link
Copy Markdown
Owner

Hello,

Not sure to understand, in which cases does the bug you mention happen exactly? Is this something you have reported upstream? Do you have a test file to provide?

Regards

@SepiRosho

Copy link
Copy Markdown
Contributor Author

Hi, thanks for looking at this.

When it happens: songrec aborts with memory allocation of ~5727255148 bytes failed (exit 134) on some input files. It depends only on the number of samples, not on the audio content or the codec. With 48 kHz input, every file whose sample count is ≡ 1 (mod 3) crashes. I tested lengths from 1 s to 30 s and all of them crashed. 44.1 kHz and 32 kHz inputs are affected at similar rates. 16 kHz input never crashes, since it doesn't go through the resampler. I originally hit it on ~10 s Opus clips from Instagram videos, where it failed on roughly a third of them.

Cause: in SignatureGenerator, UniformSourceIterator::new(decoder, 1, 16000).collect() reserves memory based on the iterator's size_hint(). Rodio's WAV decoder keeps incrementing its read counter when the resampler polls it past EOF. At the end of the stream that makes size_hint() underflow to about 2^32/3 samples, which is where the ~5.7 GB comes from. The fix collects with plain push(), so only the Vec's own growth is used.

Upstream: I haven't reported it to rodio yet, and I couldn't find an existing issue. 0.22.2 is still their latest release. I'm happy to open one there as well.

Test file: attached as a zip. It's a 9.9 s mono 48 kHz sine tone with 474067 samples. You can also generate it yourself:

import wave, struct, math
w = wave.open('songrec-crash-474067.wav', 'wb'); w.setnchannels(1); w.setsampwidth(2); w.setframerate(48000)
w.writeframes(b''.join(struct.pack('<h', int(8000*math.sin(i*0.05))) for i in range(474067))); w.close()
$ songrec audio-file-to-fingerprint songrec-crash-474067.wav
memory allocation of 5727255148 bytes failed
Aborted (core dumped)

Note: on a machine with a lot of RAM the kernel may simply grant the 5.7 GB. It's never actually touched, so nothing visibly breaks. I hit it on a 2 GB VPS. ulimit -v 4000000 should make it reproduce anywhere. With the patch it prints a normal data:audio/vnd.shazam.sig;base64,... signature and exits 0.

songrec-crash-sample.zip

@marin-m

marin-m commented Sep 29, 2026

Copy link
Copy Markdown
Owner

Thank you for the sample, it can reproduce using ulimit,

Yes it would be a good idea to report to the upstream and paste a link here to observe the followups I think.

Have you checked whether you alternate solution is not slow to run?

A lot of Linux desktop users are AI-averse so I'm not sure if I want to merge a PR with Claude as a co-author in this repository (this may appear in the contributors list in the page of this project). I may accept directly a patch written without it or rewrite a patch without it though.

@marin-m

marin-m commented Oct 5, 2026

Copy link
Copy Markdown
Owner

(I erroneously erased the commit trying to edit it, I'm sorry, the diff is still available at e4d049b)

@marin-m

marin-m commented Oct 5, 2026

Copy link
Copy Markdown
Owner

Reopened at #343

@SepiRosho

Copy link
Copy Markdown
Contributor Author

I understand your concern about merging because of the use of AI. The whole point of this pull request was to let you know this issue exists and maybe add a temporary fix on it in anyway you see fit. I don't have a problem if you want to manually write the fix outside this PR so feel free to do that. Also for that question you asked about speed, I haven't ran any specific tests for it but I'm currently using my edited version in production and haven't seen any speed related issues so far. I'll report the bug upstream and send the link back here.

@SepiRosho

Copy link
Copy Markdown
Contributor Author

RustAudio/rodio#929

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.

2 participants