While working on a solution for bug #796692, I looked at the locking functionality in more detail and came to the conclusion that we better get rid of it for 2.0, and ideally for 1.0 too in one way or another (see below).

To shortly reiterate what it is for and how it works:
- it decouples reference count from writability of the objects
- it requires explicit lock/unlock calls with the EXCLUSIVE flag whenever something "takes ownership" of the object. This is basically a reference count again, that does not decide over the object lifetime. 0/1 means object is writable, > 1 means it is not
- it contains a second component which is the read/write mapping part. Once locked readable, only readable locks are possible, once writeable/readwrite all is possible. And write locks require the object to have an exclusive count of 0 or 1 to succeed

So in short: we have a second reference count for deciding writability, and at the same time keep track of read/write mapping of the object. In an atomic way.

This was introduced for bindings: apart from C++/Rust (and possibly some other languages) you don't have explicit control over the reference counting so could accidentially end up with non-writable objects although they should be writable, or you could share the object in multiple places but the refcount is still 1.

----

So why I believe we should get rid of this is the following:

1) Nobody outside gstmemory.c, gstbuffer.c and gstminiobject.c is using these functions but everybody who is doing anything with GstMemory should to modify the writability of the object correct

2) Bindings can't make automatic use of this (except for C++/Rust possibly), and nowadays don't, and also no user-code in bindings is making use of these functions. So the original goal was clearly not achieved

3) It is only used for GstMemory right now and we can't use it for anything else existing because that would break ABI. So even if it was useful, its usefulness would have no effect because most code does not work with GstMemory but other things

4) It's complicated! The implementation is (it involves an atomic spinlock among other things), but more so the concept is. That nobody currently calls that API although they should, should be enough proof of that. Many people already struggle with reference counting, and this adds yet another unfamiliar concept that is not reference counting but almost but not really

5) Current code seems to assume reference count based writability, leading to annoying bugs (if you assume reference counting, your objects are always writable although they shouldn't).

6) It does not bring any advantage compared to using the reference count for writability (for the bindings case see above). If reference count == 1, you can write and keep track of that in an instance variable. If not, you can't. Only the check for the reference count has to be atomic, the other part not. The only theoretical advantage of mixing both things is that you currently can't increase the "exclusive count" if the object is write-locked. The only place that actually checks for this to fail is in gstbuffer.c (all other code silently fails and worse things happen), and even there it is not much of an advantage: what it means is that someone has the memory write mapped, there might be 1000 other places referencing the memory and one of those gave the memory to you and now you want to write map it too. So the only thing that this can do right now is to copy the memory, but the memory is write-mapped and some other thread might do whatever it wants with the content while we're copying it. There's not much gained here.

In this case the original bug is that a write-mapped memory actually has multiple references, and that should've never happened to begin with. Anything that is done from that point onwards is just mitigation, and not very effective.

----

My proposal is to disable the locking functionality by default (make it noops and use the reference count instead, which is what code currently assumes anyway), use the struct field for keeping track for memory mapping only, and only fall back to the current behaviour if an environment variable is used.

I didn't find any code on the Internet that is making use of the API, but quite some code that should make use of it (our own code among other things, whenever dealing with GstMemory) and would potentially have subtile bugs now that would be solved by switching to reference counting instead.


Any opinions, anything I'm missing here?