🔒 Locked

ThreadLock

Started by Daggerbot Dec 2, 2008 at 5:18 PM 2 replies 650+ views

Original Post

Dec 2, 2008 at 5:18 PM

This class is inherited by any other class to make it thread-safe. I was wondering how thread-safe this code really is. Any help is appreciated.

class ThreadLock
{
  volatile bool locked_;

public:
  // unlocked upon instantiation
  ThreadLock()
  {
    locked_ = false;
  }

  // waits for a turn and locks the object
  void Lock()
  {
    while( locked_ );
    locked_ = true;
  }

  // returns true on success, or false on timeout
  bool Lock( double timeout )
  {
    clock_t timeout2 = clock_t( timeout * CLOCKS_PER_SEC );
    clock_t start = ::clock();

    while( locked_ )
      if( ::clock() - start > timeout2 )
        return false;

    locked_ = true;
    return true;
  }

  // unlocks the object
  void Unlock()
  {
    locked_ = false;
  }

  // returns the status of the lock
  bool IsLocked() const
  {
    return locked_;
  }
};

Antheus

216

Antheus

Dec 2, 2008 at 6:05 PM

Quote:
I was wondering how thread-safe this code really is.

I don't believe it's possible to safely implement spinlock without xchg of sorts.

In following case, multiple threads can acquire the lock on context switch.

while( locked_ )  if( ::clock() - start > timeout2 )        return false;// context switch    locked_ = true;    return true;

Another problem is that for single core CPUs the while loops would need to have Sleep(x) in them, or the wait loop may prevent other threads from progressing. While it shouldn't be a problem in general, reading a volatile variable may also stress the memory bus, but that's hardware specific.

At very least, the code of lock should be:

int lock_value;...int new_value = 1;while (exchange(lock_value, new_value) != 0) Sleep(x);

Where exchange is the appropriate atomic exchange operation.

X is also problematic, since the value could be 0 on 1, depending on how scheduler would handle value of 0 - it depends whether it schedules a different thread.

Anon Mike

117

Anon Mike

Dec 2, 2008 at 6:08 PM

  void Lock()  {    while( locked_ );    locked_ = true;  }

1. Thread locks the object
2. Two other threads are spinning in the while loop
3. First thread unlocks
4. Second thread drops out of the loop
5. OS context switches to the third thread before setting locked to true
6. Third thread drops out of the loop
7. Both threads think they've locked the object

I'm not even going to start on what exactly "volatile" means in this context or what will happen with processors that play fast&loose with cache coherency.

Syncronization primitives are much harder than they look. Your OS will have some written already. Prefer to use those.

KulSeran

196

KulSeran

Dec 2, 2008 at 6:22 PM

A quick repeat From Here.

//------------------------------------------------------------------------class Mutex{public:	Mutex()	{		InitializeCriticalSection(&m_mutex);	}	~Mutex()	{		DeleteCriticalSection(&m_mutex);	}	void Acquire()	{		EnterCriticalSection(&m_mutex);		}	void Release()	{		LeaveCriticalSection(&m_mutex);	}private:	CRITICAL_SECTION m_mutex;}//------------------------------------------------------------------------// A Lock provides a safe way to acquire and release a Mutex. The Mutex // is acquired when the Lock it created. The Mutex is released when the// Lock goes out of scope.//------------------------------------------------------------------------class Lock{public:	Lock(Mutex &mutex) : m_mutex(mutex) {m_mutex.Acquire();}	~Lock() {m_mutex.Release();}	const Lock &operator = ( const Lock &l ) { l; return *this; }private:	Mutex &m_mutex;}

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.