gst: fix aravissrc teardown deadlock under incomplete frames - #1087
Open
MrBlenny wants to merge 1 commit into
Open
gst: fix aravissrc teardown deadlock under incomplete frames#1087MrBlenny wants to merge 1 commit into
MrBlenny wants to merge 1 commit into
Conversation
gst_aravis_create() waited for a buffer while holding GST_OBJECT_LOCK, and the element registered no GstBaseSrc unlock() vmethod, so the base class could not interrupt the wait. Whenever complete frames stopped arriving (high frame-rate, packet loss, or bandwidth contention from many cameras) the stream delivered a steady flow of non-SUCCESS buffers and create() spun on them forever, so a state change to NULL deadlocked trying to take the object lock. - add unlock()/unlock_stop() that set an atomic "flushing" flag - create() now refs the stream and drops GST_OBJECT_LOCK across the buffer wait, polls in short slices honouring the flushing flag (returning GST_FLOW_FLUSHING), and errors out after buffer_timeout_us instead of spinning on failure buffers - start() clears the flag so a reused element streams again Add a gstreamer test suite (tests/arvgstteardowntest.c) that tears down an aravissrc pipeline fed by a lossy in-process fake GV camera; it deadlocks without the fix. Built only when the plugin is, and excluded from the valgrind setup like the python suite.
Contributor
|
Hi, Thanks for the patch. Not an issue, but did you use AI to cook this one up ? The regression test is welcome. But it fails for me: While running the test by hand works: Any idea why ? |
Contributor
|
CI pipelines all fail |
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.
Problem
Setting a pipeline containing
aravissrctoNULLcan hang indefinitely. It shows up when complete frames stop arriving - high frame rate, GigE packet loss, bandwidth contention from several cameras, or an unplugged camera and gets worse with more cameras / higher frame rate.Root cause
gst_aravis_create()waits for a buffer while holdingGST_OBJECT_LOCK, and the element registers noGstBaseSrcunlock()vmethod, so the base class has no way to interrupt the wait during a state change. When the stream only yields incomplete (non-SUCCESS) buffers, thedo { … } while (status != SUCCESS)loop spins on them forever and never releases the lock, so thePAUSED→READYtransition deadlocks waiting for the streaming task to pause / for the lock.Fix
unlock()/unlock_stop()that set/clear an atomicflushingflag.create()now takes a ref on the stream and dropsGST_OBJECT_LOCKacross the buffer wait, polls in short slices (GST_ARAVIS_BUFFER_POLL_SLICE_US) checking the flag and returningGST_FLOW_FLUSHINGpromptly on unlock.buffer_timeout_ustotal; on continuous failure buffers it now returnsGST_FLOW_ERRORinstead of spinning.start()clears the flag so a reused element streams again.Behaviour changes (intentional)
GST_FLOW_ERRORafterbuffer_timeout_usrather than an unbounded spin.create()time returnsGST_FLOW_FLUSHINGinstead of hitting anARV_IS_STREAMg_return_val_if_failCRITICAL +GST_FLOW_ERROR.Test
New
gstreamertest suite (tests/arvgstteardowntest.c): an in-process fake GV camera with a highgvsp-lost-ratiofeedsaravissrc ! fakesink; the test assertsset_state(NULL)completes within a bound. It aborts (deadlock) without the fix and passes with it. Built only when the plugin is enabled; excluded from the valgrind setup like thepythonsuite.subdir('gst')now runs beforesubdir('tests')so the suite can link the plugin.Note: I built and ran this against 0.9.3-dev in a container - meson test --suite gstreamer fails (SIGABRT) without the fix and passes with it; a normal 25 fps stream still delivers frames with clean EOS.