GNOME Bugzilla – Bug 724743
Some tweaks re: GRWLock
Last modified: 2018-05-24 16:18:36 UTC
As I was using some GRWLock-s I came to realize a few things. First of all, the doc is a bit unclear, e.g. using "any thread" or "any other thread" to refer to the same thing, even though that's not really actually saying the same thing. I'll attach a patch that addresses this, and also makes it clear that trying to lock while already owning a lock is undefined behavior (except for read locks). This is important especially since one might have expected to get a deadlock, as the doc states it "blocks until" it gets the lock, while in reality it might just return right away without the lock, giving no hints whatsoever that something wrong happened (and will happen again when trying to unlock a lock you don't have). Speaking of, I'm also adding a patch that adds critical warnings in case of EDEADLK, since that means there's a bug somewhere. Also, as I was reading the doc about pthreads, I think even with those changes things aren't quite right in GLib, because g_rw_lock_reader_lock() claims to block until it gets the lock, but that's not true: it could fail if the maximum number of read locks has been exceeded. I wonder if something shouldn't be done as well, e.g. make it return a gboolean, whether the lock has been aquired or not. Otherwise it's a condition which isn't really a bug in the application, yet there's no way to actually know if a read lock is owned or not, which will lead to troubles. Making it return a gboolean wouldn't break any existing code, but allow future code to make sure a read lock is held, and if not error out or something. (Note that I have no idea how things work on Window however: could the same be done, or would it then always return TRUE?)
Created attachment 269719 [details] [review] GRWLock: Tweak doc to make things a bit clearer The doc used different phrasing for the same thing, e.g. "if any thread" vs "any other thread." Also make it clear that trying to take a write lock while already having a lock, or trying to take a read lock while having a write lock, is undefined.
Created attachment 269720 [details] [review] GRWLock: Tweak re: blocking on g_rw_lock_reader_lock() There's no guarantee that it will block if another thread is blocking for a write lock. See https://bugzilla.gnome.org/show_bug.cgi?id=681980
Created attachment 269721 [details] [review] GRWLock: add critical warnings for EDEADLK If failing to get a read or write lock due to deadlock condition detected, i.e. there's a bug somewhere, emit a critical warning. Can be useful, especially since otherwise there's no way of knowing we don't actually have a lock, even though the doc says the function either gets it or waits (deadlock) for it.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/832.