Re: Threads: New Win32 Implementation

From: William Kempf (sirwillard_at_[hidden])
Date: 2000-09-08 10:23:06

--- In boost_at_[hidden], "Bill Wade" <bill.wade_at_s...> wrote:
> > From: William Kempf [mailto:sirwillard_at_m...]
> 
> > > > Uhmmm... I'd have to see a concrete example.  Other than a 
slight
> > > > performance hit I can think of no reason why the locks must
> > overlap
> > > > this way.
> 
> Suppose I want to implement wait()?

Uhmmm... wait() is already provided, so your stretching things here.  
However, I'll take the question at face value assuming you'll want to 
code some alternate wait().

>  To paraphrase, the current windows
> sample code has:
> 
> wait(Lock& lock)
> {
>   ...
>   lock.mutex.unlock()
>   ...
>   lock.mutex.lock();
>   ...
> }
> 
> Strictly speaking there is no overlap involved here.  However, I 
believe
> that you're suggesting that if I want to write wait() using the 
boost
> interface I should use something like
> 
> wait(auto_ptr<Lock>& lock, Mutex& mutex)
> {
>   ...
>   lock.release();
>   ...
>   lock.reset(new Lock(mutex));
>   ...
> }
> 
> Not only does this version of the code have a "slight performance 
hit", but
> the exception safety may also be compromised.  It is plausible that 
the
> first version always succeeds.  Clearly the second version may fail 
because
> of a failure to allocate memory.  The second version might also 
change the
> dynamic type of *lock (say from a LoggingLock to a plain old Lock).

The second argument is a non-argument, since the lock classes aren't 
designed for polymorphism.  The first argument at first sounds like a 
valid argument, but upon closer scrutiny it doesn't hold.  Here's a 
quick "unsafe_lock" template to allow for the functionality being 
requested using nothing but the current boost interface.  Note that 
there's no performance hit from allocation nor any more chance for an 
out of memory exception than the original wait().

template <typename M>
class unsafe_lock
{
private:
        unsafe_lock(const unsafe_lock&);
        operator=(const unsafe_lock&);

public:
        unsafe_lock(M& mx) : m_mx(mx), m_locked(false) { lock(); }
        ~unsafe_lock() { unlock(); }

        void lock() {
                if (m_locked)
                        return;	// Throw?
                new ((void*)m_lock) M::lock(m_mx);
                m_locked = true;
        }
        void unlock() {
                if (!m_locked)
                        return; // Throw?
                ((M::lock*)m_lock)->~basic_lock<M>();
                m_locked = false;
        }
        M::lock& get_lock() {
                if (!m_locked)
                {
                        // I have to throw here.  std::bad_cast
                        // is a poor choice, but it's example code.
                        throw std::bad_cast();
                }
                return *(M::lock*)m_lock;
        }

private:
        M& m_mx;
        bool m_locked;
        char m_lock[sizeof(M::lock)];
};

The name of this lock and the fact that to use it with a CV requires 
an extra call to get_lock() are enough that I might even consider 
inclusion of such a lock within the final library.  I'm dead set 
against this "unsafe_lock" being the interface for the actual lock, 
or exposing lock/unlock methods on the mutex, however.

Hopefully this is enough to convince everyone that there isn't really 
an issue with the current interface.

Bill Kempf