GNOME Bugzilla – Bug 762216
rtpsession: don't lock while emitting the stats signal
Last modified: 2016-05-20 06:26:56 UTC
Created attachment 321537 [details] [review] test and fix Keeping the lock while emitting the stats signal introduces potential deadlock in those situations when the signal callback wants the access to rtpsession's properties which also requre the lock.
Similar issue to bug #762135 btw, might make sense to do a general review of the signalling in rtpbin and related code.
This was fixed independently without test now, sorry! commit fe34f46f32db15b62b445e82d3c7191f682ddf8b Author: Sebastian Dröge <sebastian@centricular.com> Date: Sun May 15 12:30:50 2016 +0300 rtpsession: Take the lock already when reading the other stats, not just for the hash table https://bugzilla.gnome.org/show_bug.cgi?id=766025 commit 204a86af97d039ddc71cc61bb8bf8a30827c1db1 Author: Sebastian Dröge <sebastian@centricular.com> Date: Wed May 11 09:28:13 2016 +0300 rtpsession: Don't notify about stats property changes while taking the session lock The signal handlers might want to actually get the value of the stats property, which would take the session lock again and deadlock. This was introduced by 2e960e70750a0cb7e1117d0c09d08597866a29ee. https://bugzilla.gnome.org/show_bug.cgi?id=766025 commit 2e960e70750a0cb7e1117d0c09d08597866a29ee Author: Miguel París Díaz <mparisdiaz@gmail.com> Date: Thu May 5 14:18:21 2016 +0200 rtpsession: Take session lock when creating stats The access to the session hash table must happen while the session lock is taken, otherwise another thread might modify the hash table while we're creating the stats. https://bugzilla.gnome.org/show_bug.cgi?id=766025
Created attachment 327921 [details] [review] rtpsession: Add test for locking of the stats signal Keeping the lock while emitting the stats signal introduces potential deadlock in those situations when the signal callback wants the access to rtpsession's properties which also requre the lock.
Review of attachment 327921 [details] [review]: ::: tests/check/elements/rtpsession.c @@ +699,3 @@ + /* then ask explicitly to send RTCP now */ + g_signal_emit_by_name (internal_session, "send-rtcp-full", 0, &ret); + fail_unless (ret == TRUE); This assertion fails
Created attachment 327959 [details] [review] rtpsession: Add test for locking of the stats signal The test should be fixed now. I've also refactored it bit.
Attachment 327959 [details] pushed as fa1c711 - rtpsession: Add test for locking of the stats signal