Created attachment 96712 [details] [review]
Implements the requested feature

I don't think the "typedef GMutex CType" is necessary and I don't think we do that elsewhere. It doesn't really even fit with the rest of the code in this class.

More importantly, I worry that you might need to think about ownership of the wrapped type. This will currently call g_mutex_free() in the destructor, but that's maybe not what you want if you are wrapping the result of something_get_mutex() with the something keeping ownership. You might want a "bool take_ownership" parameter in the constructor, as we do in some other classes. However, I think that would need a new boolean m_has_ownership member, breaking ABI.

If so, and if there is no way to associate a boolean with the underlying GMutex, you might need to derive your own MutexWithoutOwnership (or better named) that you can use in those cases. You'd have to make sure that application coders are never expected to delete it via the Glib::Mutex class, because the Glib::Mutex destructor is not virtual.

 

Thoughts?

I had the same idea regarding ownership with a MutexWithoutOwnership class, but this would be like you said a bad choice regarding the virtual dtor, which can be changed in a later release of glibmm but then we could as well make the change of a bool arg to Glib::Mutex regarding the ownership, so it all hinges on breaking the ABI.

Adding such a class temporarily would be, needless to say, very bad, since people would incorporate this in their code, and finally when a bool arg to Glib::Mutex(GMutex*) is introduced, well, I'll just stop here as it can be clearly seen that this is just BAD! :)

I'd argue for introducing the boolean parameter at the next ABI break, so far I have no better idea.

Glib::Mutex is now deprecated. It has been replaced by Glib::Threads::Mutex.

Glib::Threads::Mutex contains
  GMutex gobject_;
instead of
  GMutex* gobject_;
which makes it impossible, I suppose, to add a constructor
  Mutex(GMutex* object, bool take_ownership);

It would be possible to add

  Glib::Threads::Mutex* Glib::Threads::wrap(GMutex* gobject)
  {
    return reinterpret_cast<Glib::Threads::Mutex*>(gobject);
  }

in analogy with

  Glib::Threads::Thread* Glib::Threads::wrap(GThread* gobject)
  {
    return reinterpret_cast<Glib::Threads::Thread*>(gobject);
  }

Is this a good or a bad idea?

What about "Glib::Threads::RecMutex* Glib::Threads::wrap(GRecMutex* gobject)"?

Is this functionality still needed, or is this bug obsolete?
I don't find the link in comment 0 very informative. Perhaps it does not link
to the same information as it did when the bug was filed?

Adding José Alburquerque to the CC list.
This bug is said to be related to gstreamermm.
You are perhaps the right person to tell if it's obsolete or not?

(In reply to comment #5)
> Is this functionality still needed, or is this bug obsolete?

The one member of the GstObject struct that this functionality would make accessible in C++ (via a getter method of the Gst::Object class) is the "lock" member which the GST_OBJECT_GET_LOCK() macro gets in C:
http://developer.gnome.org/gstreamer/0.10/GstObject.html#GST-CLASS-GET-LOCK:CAPS

I guess getting the lock would probably be useful to C++ programmers so this bug should still be addressed.

The correct link to the GST_OBJECT_GET_LOCK() macro is:
http://developer.gnome.org/gstreamer/0.10/GstObject.html#GST-OBJECT-GET-LOCK:CAPS

Created attachment 217232 [details] [review]
patch: Glib::Threads:[Rec]Mutex: Add wrap() functions

This patch adds wrap() functions for GMutex and GRecMutex, similar to the
wrap() function for GThread.

Only a wrap() function for GMutex is requested in the bug, but it seems
reasonable to treat GRecMutex in the same way as GMutex.
I have not made const versions of wrap(). A constant mutex is not very useful,
is it?

Does the patch in comment 8 give gstreamermm what it needs?

Are those wrap() functions acceptable in glibmm?
It's an unusual kind of wrap() function. I think such wrap() functions are
possible only for a C++ class that contains exactly one non-static data member
and no virtual functions.

(In reply to comment #10)
> comment 8

Shall be: comment 9.

José, could you respond please.

However, it does seem worth checking if this is still appropriate in GStreamer 0.11 (git master, which will become GStreamer 1.0).

Sorry for the delay; A few other things have required my attention.

The patch does provide gstreamermm with a way to address the issue.  Locking functionality is still present in GStreamer 0.11 so it would still be appropriate to address this for the future.

I have pushed the patch in comment 9.