tags:

views:

896

answers:

3

I know, I know, the title of my message may seem provocative, since boost::mutex purposefuly do not expose lock / unlock (in order to avoid dead locks).

However the boost documentation is quite short on these aspects (to say the least), so I am asking if anyone can help me in the following use case.

Suppose you have a class Foo, which has :
- a destructor that takes some time to complete
- a method that is called by a distinct thread, but should not be called during destruction

class Foo
{
 public:

  virtual ~Foo()
  { 
    //Time consuming operations here
  }


  //Method called by a timer belonging to a distinct class 
  void OnTimer()
  {
    //Other time consuming stuff. Should not be called during destruction !
  }  


};

I tried (with no success) to implement a version based on boost::mutex

//boost::mutex implementation
class Foo
{
public:
  Foo()
  {
  }

  virtual ~Foo()
  { 
    {
      boost::mutex::scoped_lock lock(mDisposingMutex);
      //Time consuming operations here
    }
  }


  //Method called by a timer belonging to a distinct class 
  void OnTimer()
  {
    {
      //Imaginary code here: mutex::locked() method is private !!!
      if ( ! mDisposingMutex.locked()) 
        return;
    }    
    //Other time consuming stuff. Should not be called during destruction !
  }  

private:
  boost::mutex mDisposingMutex; 
};

Am I totally wrong? Can anyone tell me how this is supposed to be done with boost::mutex?

Thanks !

+1  A: 

mutex::try_lock()

rad
+3  A: 

If you do commit to using Lockable::lock() in the destructor body, you could have your OnTimer() function use Lockable::try_lock(), and proceed only if that function returns true. That will have OnTimer() put the destructor on hold if OnTimer() starts first, but it still doesn't solve the problem of the destructor running, finishing and releasing the mutex, and then OnTimer() starting and successfully grabbing the mutex.

Such a sequence is likely in the realm of undefined behavior, but that curse won't stop it from happening. Using a state flag in addition to the mutex -- similar to what I described in my comment above -- could allow you to detect this latter case and stop OnTimer() from doing anything beyond reading the flag. At some point, though, this is just putting Band-Aids on top of Band-Aids.

seh
A: 

@Seh : I totally agree this is a code smell and I should (and will) correct the root cause.

However, in the hope of helping anyone who would encounter the same problem as me (i.e fighting with the boost doc), I tried to implement your suggestion. The code below now compiles correctly (though the code smell is now very strong)

#include <boost/thread/mutex.hpp>

//boost::mutex implementation
class Foo
{
public:
  Foo() :
    mIsDisposing(false)
  {
  }

  virtual ~Foo()
  { 
    {
      boost::try_mutex::scoped_try_lock lock(mDisposingMutex);
      if ( ! lock.locked())
      {
        //Die by horrible death, or wait before trying again...
      }
      else
      {
        mIsDisposing = true;
      }
      //Time consuming operations here
    }

  }


  //Method called by a timer belonging to a distinct class 
  void OnTimer()
  {
    {
      boost::try_mutex::scoped_try_lock lock(mDisposingMutex);
      if ( ! lock.locked() || mIsDisposing )
      {
        return;      
      }
    }    
    //Other time consuming stuff. Should not be called during destruction !
  }  

private:
  boost::try_mutex mDisposingMutex; 
  bool mIsDisposing;
};
Pascal T.