RE: [boost] Re: Boost.Threads: Do we need all those mutexes?

From: Howard Hinnant
Exploring the case: throw exception if unable to atomically promote read to write:
If an exception is not thrown, then you are guaranteed what you previously read is still true, and all is well. If an exception is thrown, then you have to re-read to determine current state.
First, to be clear, a promote() _must_ not be allowed to silently fail, otherwise you break the contract and people will spend several days tracking down the problem. If you want that behavior you should use try_promote(). I don't have a problem with it simply deadlocking, or, if that's unacceptable don't have the blocking promote() function. The complexity involved in determining a deadlock may be well beyond what you want to do at this level in the library, I'll come back to this later. But for now, lets talk about throwing. In my mind the purpose of throwing is to allow the other thread (or threads, with the database work I've been doing multiple read locks on an object all trying to be promoted to write locks at the same time happens more often then you might think) to proceed, which it can't do unless you give up your _read_ lock.
Hmm...
Given that when you have write privilege you also implicitly have read privilege, this seems a little extreme. Would it not be both more efficient and easier to follow if you just reaffirm your state under the write lock rather than assume your state, and write a catch block to reassert it if it happens to be false? That is, this:
bool try_sale(rw_mutex& m) { read_lock rl(m);
long balance = get_balance(); long cost = get_total_cost();
if (balance > cost) { write_lock wl(rl); set_balance(balance - cost); // ... return true; } return false }
bool do_sale(rw_mutex& m) { while (true) { try_again: try { return try_sale(m); catch (rw_promotion_error&) { // Try the whole thing again // how is this implemented? goto try_again; // ? } } }
vs this:
void do_sale(rw_mutex& m) { read_lock rl(m);
long balance = get_balance(); long cost = get_total_cost();
if (balance > cost) { rl.unlock(m); write_lock wl(m); set_balance(balance - cost); wl.transfer_to_read_lock(rl); balance = get_balance(); } // ... }
My assertion is that you don't know a-priori if the read-to-write promotion achieves anything or not. If you allow it to fail, by exception, or by error code, or by any other means, then you have to write code to re-read the current state. If you don't allow it to fail, then you must make it block until it succeeds. The latter seems simpler in logic, and smaller in code size. The logic is quite simple:
read_lock to write_lock promotion is not atomic.
Once that is understood, the programmer can just deal with it. Trying to encapsulate this bit of reality only serves to obfuscate the necessary logic.
The reason that "release read, acquire write" appears a complete alternative to promotion is because your example is too simple. You may need the read lock to keep the object alive in a cache, where it could be flushed to disk and destroyed if you release it, forcing you to re-read it before getting your write lock. I think that the library should either commit to detecting deadlocks or don't do it at all. Is it going to throw a promotion exception in this case? Thread A: read lock x Thread A: read lock y Thread B: read lock y Thread B: write lock x, blocks Thread A: promote y to write lock, blocks In order to detect things like this you essentially need to build a complete conflict matrix of all held locks in the system, and if you do that I assume you'd also use it to detect deadlocks of regular locks: Thread A: lock x Thread B: lock y Thread A: lock y, blocks Thread B: lock x, blocks Glen

Glen Knowles wrote:
From: Howard Hinnant
Exploring the case: throw exception if unable to atomically promote read to write:
If an exception is not thrown, then you are guaranteed what you previously read is still true, and all is well. If an exception is thrown, then you have to re-read to determine current state.
First, to be clear, a promote() _must_ not be allowed to silently fail, otherwise you break the contract and people will spend several days tracking down the problem. If you want that behavior you should use try_promote().
I don't have a problem with it simply deadlocking,
I do however: it would make lock promotion unusable. It is possible to use other lock operations to cause deadlock, but only by using them incorrectly. In this case deadlock would result from the correct use of the lock, which is unacceptable.
or, if that's unacceptable don't have the blocking promote() function.
I do agree with this.
The complexity involved in determining a deadlock may be well beyond what you want to do at this level in the library, I'll come back to this later.
Given one assumption, it's not really that hard; see below.
But for now, lets talk about throwing. In my mind the purpose of throwing is to allow the other thread (or threads, with the database work I've been doing multiple read locks on an object all trying to be promoted to write locks at the same time happens more often then you might think) to proceed, which it can't do unless you give up your _read_ lock.
Yes. [snip examples]
My assertion is that you don't know a-priori if the read-to-write promotion achieves anything or not. If you allow it to fail, by exception, or by error code, or by any other means, then you have to write code to re-read the current state. If you don't allow it to fail, then you must make it block until it succeeds. The latter seems simpler in logic, and smaller in code size. The logic is quite simple:
read_lock to write_lock promotion is not atomic.
Once that is understood, the programmer can just deal with it. Trying to encapsulate this bit of reality only serves to obfuscate the necessary logic.
The reason that "release read, acquire write" appears a complete alternative to promotion is because your example is too simple. You may need the read lock to keep the object alive in a cache, where it could be flushed to disk and destroyed if you release it, forcing you to re-read it before getting your write lock.
I think that the library should either commit to detecting deadlocks or don't do it at all. Is it going to throw a promotion exception in this case?
Thread A: read lock x Thread A: read lock y Thread B: read lock y Thread B: write lock x, blocks Thread A: promote y to write lock, blocks
In order to detect things like this you essentially need to build a complete conflict matrix of all held locks in the system, and if you do that I assume you'd also use it to detect deadlocks of regular locks:
As I said above, it's not that hard, really, given one assumption. You just prevent more than one thread from waiting for promotion at a time. The assumption is that threads that fail to be promoted don't loop and try again without ever releasing their read lock.
Thread A: lock x Thread B: lock y Thread A: lock y, blocks Thread B: lock x, blocks
Mike
participants (2)
-
Glen Knowles
-
Michael Glassford