GNOME Bugzilla – Bug 767688
queue2: fix crash deleting current region for small ring buffers
Last modified: 2016-11-22 11:43:34 UTC
repro case: gst-launch-1.0 filesrc location=sample.ogg ! queue2 ring-buffer-max-size=1000 ! oggdemux ! opusdec ! autoaudiosink
Created attachment 329852 [details] [review] queue2: fix crash deleting current region for small ring buffers
Could you add a unit test as well? :)
Will do :)
Created attachment 329866 [details] [review] unit test for regression
Comment on attachment 329852 [details] [review] queue2: fix crash deleting current region for small ring buffers Please explain a bit in more detail what happens, why it crashes and under what conditions, and how that is solved :) In the commit message I mean. What would be the effect of create_write() returning FALSE? Shouldn't it rather create a new range instead?
Created attachment 329935 [details] [review] queue2: fix crash deleting current region for small ring buffers Now with more comments in the commit message. The current behavior is to fail. Maybe creating another range would be better, I don't know how queue2 works internally. But I suspect the reason why the current range is destroyed is because there is no space for a new one.
I guess this needs further investigation then to understand what should actually happen, and if failing is the right thing to do.
Created attachment 329957 [details] [review] queue2: fix crash deleting current region for small ring buffers
Created attachment 329958 [details] [review] unit test for regression
In fact, it turns out that simply not destroying the current range works, and does not need erroring out. So it was not about no space left after all.
commit 08d30b05c63cfef7244e02ba04ff9de07f4ced3c Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Wed Jun 15 16:24:27 2016 +0100 tests: add a test for small ring buffer sizes https://bugzilla.gnome.org/show_bug.cgi?id=767688 commit b3802f7a9e7988901367196dd3dc45cf4053d850 Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Wed Jun 15 13:43:59 2016 +0100 queue2: fix crash deleting current region for small ring buffers Ensure we do not attempt to destroy the current range. Doing so causes the current one to be left dangling, and it may be dereferenced later, leading to a crash. This can happen with a very small queue2 ring buffer (10000 bytes) and 4 kB buffers. repro case: gst-launch-1.0 fakesrc sizetype=2 sizemax=4096 ! \ queue2 ring-buffer-max-size=1000 ! fakesink sync=true https://bugzilla.gnome.org/show_bug.cgi?id=767688
Should this go into 1.8?