After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 387385 - Wish: g_static_rec_mutex_get_depth()
Wish: g_static_rec_mutex_get_depth()
Status: RESOLVED WONTFIX
Product: glib
Classification: Platform
Component: general
2.12.x
Other Linux
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks: 356177
 
 
Reported: 2006-12-19 02:51 UTC by Matthew Barnes
Modified: 2007-01-10 17:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (2.92 KB, patch)
2006-12-19 02:53 UTC, Matthew Barnes
none Details | Review

Description Matthew Barnes 2006-12-19 02:51:17 UTC
I'd like to propose a new function to allow applications to peek at the depth of a GStaticRecMutex:

   guint g_static_rec_mutex_get_depth (GStaticRecMutex *mutex)

Returns the number of times "mutex" has been locked by the current thread.  If "mutex" is unlocked or was locked by another thread, the function returns 0.


The motivating use case for this is bug #356177, where I'm attempting to migrate Evolution-Data-Server off its own EMutex data structure to GLib's GStaticMutex and GStaticRecMutex.  (EMutex is a combination simple/recursive mutex, hand-rolled in a time before GLib offered recursive mutexes.)

EMutex maps cleanly to the functionality now offered by GLib except for one case.  The EMutex API offers a function called e_mutex_assert_locked() which asserts that a recursive EMutex is owned by the current thread.  GLib offers no (documented) way to simulate this.

The proposed function for GLib is more general-purpose than what is required for the aforementioned use case, but does make it possible to assert ownership:

   /* Make sure the current thread owns the mutex. */
   g_assert (g_static_rec_mutex_get_depth (mutex) > 0);
Comment 1 Matthew Barnes 2006-12-19 02:53:04 UTC
Created attachment 78609 [details] [review]
Proposed patch
Comment 2 Matthias Clasen 2006-12-19 03:38:01 UTC
Hmm, 

I'm not sure I really like this. It is ugly that this functionality would be available for recursive mutexes when it would make just as much sense for non-recursive ones - however, there is no good way to implement it on top of
non-recursive pthreads mutexes.

Also, there is the larger-picture question of what good this check does. If you are using multiple threads and locking, you damn well better know at each point in your code which locks you are currently holding. If you don't, you have already lost, and these assertions will not do you much good.
Comment 3 Philip Van Hoof 2006-12-19 11:01:32 UTC
Just a little question about this one. Can't the depth itself change while the function is happening? Making the returned value unreliable.
Comment 4 Havoc Pennington 2006-12-19 15:44:33 UTC
re: comment #3, you can in theory assert you have a lock because the only way to unlock (or increase lock depth) is to do it in the thread that already has the lock, i.e. the one containing the assertion. So the "this thread has the lock" state should not be changed by another thread, though the "some thread has the lock" state may be.

re: the patch, get_depth() > 0 doesn't mean *your thread* has a lock it means *some thread* had the lock for a moment (and may have dropped it now). The assertion should probably check pthread_equal(pthread_self(), lock->holder) or something like that.

I would think g_static_mutex_assert_locked() is a better API but as Matthias says I don't know how you would implement it for nonrecursive mutexes...
Comment 5 Matthew Barnes 2006-12-19 16:33:17 UTC
Actually, Havoc, I did try to design the function so that you only get back a non-zero value if *your thread* owns the lock:

    if (g_system_thread_equal (self, mutex->owner))
        return mutex->depth;

    return 0;

If the mutex is unlocked or if another thread has already locked it, you should always get back zero.  That should make the return value reliable, to address Philip's concern.

I should try to clarify that the idea behind the proposal is to allow an application to peek at the value that g_static_rec_mutex_unlock_full() would return without changing the state of the mutex.  The fact that you can also use this to check whether you own the mutex at all is kind of a dirty hack that any well-written threading logic should already be aware of (as Matthias correctly stated).

So my EMutex use case was probably not the best motivating example.

However the next question -- "why would you need to know the depth if not to assert that your thread owns the mutex?" -- I'm finding difficult to answer.  Perhaps that's a sign that this isn't worth bothering with.

Are there other use cases for such a function?
Comment 6 Matthew Barnes 2007-01-10 17:22:52 UTC
Closing this as WONTFIX since I can't think of a legitimate use case for it.