CODEWITHSUNDEEP
windowsmultithreadingwinapicritical-section
Simon Richter
Simon Richter

Reputation: 29586

Assert that current thread does not hold a CRITICAL_SECTION lock

I have an object that maintains a list; one of the helper methods needs to

The cleanup operation removes the object from the list from the other thread, thus it needs to lock the list in between.

This works fine as long as the helper is not called with the lock on the list already held as then the unlock operation will not actually allow the other thread to access the list, so I'd like to flag an error in this case.

As far as I've understood, the CRITICAL_SECTION API does not provide an officially supported way to query whether the current process holds this object, so I'm considering "hack-ish" approaches (after all, it's a debugging aid and not intended to go into production code):

Variant 1 is to check the OwningThread field of the CRITICAL_SECTION structure, but I wonder whether there is a guarantee that this field is

Variant 2 is to lock the CRITICAL_SECTION and then examine the RecursionCount; this assumes that the recursion counter has a fixed start value.

Is there anything that I have missed that I could use to build a somewhat future-proof (that is, it will break noisily in a line of code that is near to the comments where I explain it all) assertion statement that the current thread is not the holder of a certain CRITICAL_SECTION?

Upvotes: 1

Views: 528

Answers (2)

fimbox
fimbox

Reputation: 11

I use something like that

class CriticalSection
{
private:
  CRITICAL_SECTION section_;
  unsigned int owning_thread_id_;
  unsigned int lock_count_;

public:
  CriticalSection() 
  {
    InitializeCriticalSection(&section_);
    owning_thread_id_ = 0;
    lock_count_ = 0;

  }
  ~CriticalSection()
  {
    DeleteCriticalSection(&section_);
  }

  void enter()
  {
    EnterCriticalSection(&section_);
    owning_thread_id_ = GetCurrentThreadId();
    lock_count_ ++;
  }
  void leave()
  {

    if(  GetCurrentThreadId() == owning_thread_id_ )
    {
      lock_count_ --;
      if( lock_count_ == 0 )
        owning_thread_id_ = 0;
    }

    LeaveCriticalSection(&section_);
  }

  bool tryEnter() 
  { 
    if( TryEnterCriticalSection(&section_))
    {
      owning_thread_id_ = GetCurrentThreadId();
      lock_count_ ++;
      return true;
    }
    return false;
  }

  bool isCurrentThreadEntered()
  {
    return GetCurrentThreadId() == owning_thread_id_;
  }

  int getLockCount() { return lock_count_; }
  unsigned int getOwningThreadID() { return owning_thread_id_; }

};

Upvotes: 1

David Schwartz
David Schwartz

Reputation: 182753

Make your own critical section that provides such a feature. Use it in your debug builds. In release builds, use the regular critical section.

One simple way is to use two critical sections and an owner field. Acquire works like this:

  1. Acquire first critical section.

  2. Acquire second critical section.

  3. Set owner to this thread.

  4. Release second critical section.

Release works like this:

  1. Acquire second critical section.

  2. Set owner to none.

  3. Release first critical section.

  4. Release second critical section.

Assert works like this:

  1. Acquire second critical section.

  2. Assert owner is not this thread.

  3. Release second critical section.

Update: There's a bug in the above. It mishandles this case: lock, lock, unlock, assert that we don't hold the critical section but we do. The fix is probably to keep a "lock count". You don't have to set the CS to "unowned" with the lock count. If the lock count is zero, it's unowned. So the assert paths "unowned or owner not this thread".

Upvotes: 2

Related Questions