I'm writing thread-safe class in C++. All of its public methods use locks (non-recursive spin locks). Private methods are lock-free. So, everything should be OK: user calls public method, it locks object and then does the work through private methods. But I got dead lock when a public method calls another public method. I've read that recursive mutexes are bad, cause it's difficult to debug them. So I use C's stdio way: public method Foo() only locks the object and calls Foo_nolock() to do the whole work. But I don't like these _nolock() methods. I think it duplicates my code. So I've got an idea: I'll make lock-free class BarNoLock, and thread-safe class Bar that has only one member: an instance of BarNoLock. And all Bar's methods will only lock this member and call it's methods. Is it a good idea or maybe there are some better patterns/practices? Thanks. Update: I know about pimpl and bridge. I ask about multi-threading patterns, not OOP.
+1
A:
Looks like you have reinvented the Bridge Pattern. Sounds perfectly in order.
doron
2010-08-16 17:23:01
That's what I was talking about -- bridge or pimpl, yeah.I was asking about multithreading idioms, not OOP
f0b0s
2010-08-16 17:33:07
+1
A:
I'm not sure why recursive mutexes would be considered bad, see this question for a discussion of them.
http://stackoverflow.com/questions/187761/recursive-lock-mutex-vs-non-recursive-lock-mutex
But I don't think that's necessarily your problem because Win32 critical sections support multiple entries from the same thread without blocking. From the doc:
When a thread owns a critical section, it can make additional calls to EnterCriticalSection or TryEnterCriticalSection without blocking its execution. This prevents a thread from deadlocking itself while waiting for a critical section that it already owns. To release its ownership, the thread must call LeaveCriticalSection one time for each time that it entered the critical section. There is no guarantee about the order in which waiting threads will acquire ownership of the critical section
So maybe you were doing something else wrong when you were deadlocking yourself? Having to work around not deadlocking yourself on the same mutex from the same thread with weird function call semantics is not something you should have to do.
bshields
2010-08-16 17:27:53
well, you are right and CS is recursive. My app doesn't dead lock in practice, only in theory. But I dont want to use recursive locks.
f0b0s
2010-08-16 17:29:59
I've read your link before and found that article there. http://www.zaval.org/resources/library/butenhof1.html
f0b0s
2010-08-16 17:34:08
Well if you're dead set against the recursive locks then what you're proposing sounds reasonable. Only other thing I would say is that you only ran into problems with your original design when one public function of your object called another. The obvious workaround there would be to just not do that and whenever you had shared functionality that needed to be shared factor it out into a private non-locking method. Then each public method pulls a lock and does its work and the private methods that are used don't pull locks.
bshields
2010-08-16 17:38:12
bshields: so, you advice me to put all public method's functionality into lock-free private metods: Foo() -> Foo_nolock()? That was an original design
f0b0s
2010-08-16 17:43:20
and Foo_nolock() etc. should be fine - I don't see how you get the code duplication you were worried about.
nos
2010-08-16 17:45:53
but do you think mixing all methods together is a good idea? May be decomposition is better (that was the original question)? So there will be 2 versions of class.
f0b0s
2010-08-16 17:56:32
Not all public methods, just the ones that you want to call from other public methods as that's where the problem happens. I understand this is your original design, I think it works better under the assumption that most of your public methods don't call other public methods. So you wouldn't have to have a _noLock version of every method. Using the two classes approach, you would have more boiler plate because the locking wrapper object would have to have matching signatures for all public methods and pass them through to the locking object.
bshields
2010-08-16 17:58:44
How does the _nolock variant duplicate code but the Bar/BarNoLock does not? It's just that the _nolock methods are in BarNoLock now.I think I'd go the Bar/BarNoLock route though, using the pimpl idiom. BarNoLock would become a private inner class of Bar then.
Frank
2010-08-16 18:19:54