[boost][interprocess] upgradable_mutex is semi-recursive...

The subject is misleading, but hopefully eye-catching :-) I do understand that this is not an interprocess bug, but more of a result of how upgradeable locks work. But the behavior can be dangerous, as will be seen. What I've got happening is that one process locks the mutex recursively (sharable, of course). And this works (most of the time), since it is okay to lock the mutex shareably any number of times (even if it is within the same thread). But the problem occurs when a writer (in another thread) manages to get inbetween these lockings, when we get this sequence thread A locks mutex shareably thread B waits for exclusive lock thread A waits for sharable lock Since the upgradable mutex is written so that if there is an exclusive locker waiting it doesnt let any more sharable lockers in this will deadlock. Something that explains this could be useful in the interprocess documentation. Or, maybe the problem could be solved. I see two ways: 1. Detect recursive sharable locking, so that my thread A would always get an exception when trying to lock a lock that I've already got (it is a recipe for disaster) 2. Allow recursive sharable locks. If you've got the lock it's okay to take it again. Since I'm not really a fan of recursive locking (the above locking is a mistake, really) I think I would prefer solution 1 of these two. (I'm fixing my code to not be recursive now, but I thought this might be of interest to either Ion or anyone using boost interprocess.) Cheers Lars

Lars Hagström wrote:
The subject is misleading, but hopefully eye-catching :-) I do understand that this is not an interprocess bug, but more of a result of how upgradeable locks work. But the behavior can be dangerous, as will be seen.
What I've got happening is that one process locks the mutex recursively (sharable, of course). And this works (most of the time), since it is okay to lock the mutex shareably any number of times (even if it is within the same thread). But the problem occurs when a writer (in another thread) manages to get inbetween these lockings, when we get this sequence
thread A locks mutex shareably thread B waits for exclusive lock thread A waits for sharable lock
Since the upgradable mutex is written so that if there is an exclusive locker waiting it doesnt let any more sharable lockers in this will deadlock.
Something that explains this could be useful in the interprocess documentation.
Well, upgradable::lock() has two gates, first it waits until upgradable and exclusive owners unlock. After that it waits until all readers are gone. Obviously, if readers are not leaving the lock you are not going to be able to continue. This means that writers have priority over readers. I don't mention anywhere that upgradable lock is recursive, so this is not going to work if you use it that way. An inteprocess_upgradable_recursive_mutex could be an option but it's not easy to implement.
Or, maybe the problem could be solved. I see two ways:
1. Detect recursive sharable locking, so that my thread A would always get an exception when trying to lock a lock that I've already got (it is a recipe for disaster)
That's an option but you might have infinite readers, and having a map with thread_id/lock count is hard. But placing it in shared memory (with no shared-memory allocators around) is nearly impossible. That's why I chose not to detect recursiveness.
2. Allow recursive sharable locks. If you've got the lock it's okay to take it again.
I find this difficult to implement for shared memory and expensive for intra-process (Boost.Thread) mutexes.
Since I'm not really a fan of recursive locking (the above locking is a mistake, really) I think I would prefer solution 1 of these two.
(I'm fixing my code to not be recursive now, but I thought this might be of interest to either Ion or anyone using boost interprocess.)
You've chosen option 3: don't use a non-recursive mutex as recursive. Mutexes are not required to notify these errors, precisely because sometimes is hard/expensive to detect this. I really would like to implement option 1 or 2, but I still don't see an easy solution. Sorry!
Cheers Lars
Regards, Ion

Hi,
Something that explains this could be useful in the interprocess documentation.
I don't mention anywhere that upgradable lock is recursive, so this is not going to work if you use it that way.
The difference with interprocess_mutex not being recursive and upgradable_mutex not being recursive is that if you try to lock interprocess_mutex recursively you will get a deadlock *every time*, whereas locking the upgradable_mutex recursively (lock_shareable) will work *most* of the time. This is the reason I think that this might be worth a mention in the docs. It took me a fair while to work out that this was what caused me some rare deadlocks, and having a mention in the docs may help others.
I really would like to implement option 1 or 2, but I still don't see an easy solution. Sorry!
Yes, that was what I was afraid of. I had a look at the code, but didn't really have time/patience to delve deeply into it. I was hoping that there was some easy way of doing this that was beyond my understanding of the implementation. I thought of changing the internal mutex into a recursive_mutex, but I couldn't easily work out what would happen if I did that... Anyway, if there is no easy way to "fix" this, I will stick with making sure that I don't lock recursively... Which is obviously the correct solution with the current behaviour. Lars
participants (2)
-
Ion Gaztañaga
-
Lars Hagström