GNOME Bugzilla – Bug 750172
miniobject double WRITE | EXCLUSIVE lock succeeds despite part-miniobject.txt forbidding it
Last modified: 2015-06-03 10:45:55 UTC
gst_memory_lock (mem, WRITE | EXCLUSIVE); gst_memory_lock (mem, WRITE | EXCLUSIVE); Succeeds when the part-miniobject.txt design doc suggests that this should fail: "A gst_mini_object_lock() can fail when a WRITE lock is requested and the exclusive counter is > 1. Indeed a GstMiniObject object with an exclusive counter > 1 is locked EXCLUSIVELY by at least 2 objects and is therefore not writable."
Created attachment 304320 [details] [review] miniobject: disallow a double write/exclusive lock
Review of attachment 304320 [details] [review]: ::: gst/gstminiobject.c @@ +195,3 @@ + /* shared counter > 1 and write access is not allowed */ + if ((state & GST_LOCK_FLAG_WRITE || access_mode & GST_LOCK_FLAG_WRITE) clang/gcc does not complain here with a warning to add parenthesis? Please add some around the bitwise AND before pushing :)
Created attachment 304354 [details] [review] buffer: locking memory exclusively may fail
With this change, gst_memory_init may fail to lock the parent memory exclusively and has no way to propagate that upwards. This means that calling code will have to deal with that instead (gst_memory_share and implementations come to mind).
(In reply to Sebastian Dröge (slomo) from comment #2) > + /* shared counter > 1 and write access is not allowed */ > + if ((state & GST_LOCK_FLAG_WRITE || access_mode & GST_LOCK_FLAG_WRITE) > > clang/gcc does not complain here with a warning to add parenthesis? Please > add some around the bitwise AND before pushing :) No, but it does if I add a "!= 0" on the end of them :)
Created attachment 304406 [details] [review] memory: gst_memory_share may fail to exclusively lock the parent memory This is one workaround that I though of for checking exclusivity in gst_memory_share. Not much code directly utilises the share path of GstMemory outside of GstBuffer/GstMemory. All the make check test cases pass in core, -base, -good, and -ugly.
commit e9c15d53212ff424a047e933ca9ab662ae155b56 Author: Matthew Waters <matthew@centricular.com> Date: Tue Jun 2 16:14:50 2015 +1000 memory: gst_memory_share may fail to exclusively lock the parent memory Now that locking exclusively dows not always succeed, we need to signal the failure case from gst_memory_init. Rather than introducing an API or funcionality change to gst_memory_init, workaround by checking exclusivity in the calling code. https://bugzilla.gnome.org/show_bug.cgi?id=750172 commit eb71ee40407a4dbd1016c962f8ef36b8d6b4a8b5 Author: Matthew Waters <matthew@centricular.com> Date: Tue Jun 2 00:23:37 2015 +1000 buffer: locking memory exclusively may fail Attempt to return a copy of the memory instead. https://bugzilla.gnome.org/show_bug.cgi?id=750172 commit ad4569c893e10e0e2dcaab3a076d837ae2bffa35 Author: Matthew Waters <matthew@centricular.com> Date: Sun May 31 21:25:23 2015 +1000 miniobject: disallow a double write/exclusive lock gst_memory_lock (mem, WRITE | EXCLUSIVE); gst_memory_lock (mem, WRITE | EXCLUSIVE); Succeeds when the part-miniobject.txt design doc suggests that this should fail: "A gst_mini_object_lock() can fail when a WRITE lock is requested and the exclusive counter is > 1. Indeed a GstMiniObject object with an exclusive counter 1 is locked EXCLUSIVELY by at least 2 objects and is therefore not writable." https://bugzilla.gnome.org/show_bug.cgi?id=750172