Subj : Re: Challenge: Multithreading & Synchronization To : comp.programming.threads From : doug Date : Sat May 21 2005 01:18 pm "Peter Dimov" wrote in message news:d6n5up$hv5$1@domitilla.aioe.org... > doug wrote: >> "David Holmes" wrote in >> message news:428d693c_2@news.melbourne.pipenetworks.com... > >>> mon_enter() { >>> if (mon.owner == currentThread()) >>> count++; >>> else >>> mon_enter_slow(); // blocks until monitor is free >>> >>> mon_exit() { >>> if (mon.owner == currentThread()) { >>> if (--count == 0) >>> mon_release_slow(); >>> } >>> else throw an error > >> I don't think this is safe - I think it's a disguised case of the >> double-checked locking pattern. >> Without a memory barrier before readig mon.owner, you risk reading an >> old value. > > As long as mon_release_slow unsets mon.owner (before unlocking) and > mon_enter_slow sets the owner (after locking), it seems to work. > > A thread that is holding the lock will always see its id in the owner > field > because of the release barrier in mon_release_slow and the acquire barrier > in mon_enter_slow. This makes the unset in the other thread precede its > store to mon.owner. > > A thread that does not hold the lock will never see its id in mon.owner > because of the preceding unset in mon_release_slow. It doesn't matter > whether the current value of owner becomes visible or not; it will always > be > different from currentThread() if the thread doesn't hold the lock. > > Yeah, you're right. So I take it all back! As long as the owner of the mutex is initialised to an invalid thread id, and this is propagated to the other threads before their first acquire (for example, the semaphores are created in the main thread before others are created), this code will work. Thanks for posting that. In particular, thanks for taking the time, instead of just blaming it on magic somewhere. (Out of interest, the bug I fixed a few months back was in code that looked identical to this, except the monitor had the property that anyone could unlock it. (The reasons for this are too boring - but it was an app-specific thing and the designer had decided to do it using some recursive-seignal-semaphore hybrid. The code was similar to that you've posted above, expect the exit case was something like (from memory): >>> if (count == 1){count = 0; mon_release_slow();} something like that. was very silly. Worked on UP, but broke every now and then on SMP, for obvious reasons!) Doug .