How to safely delete member std::mutex?

Question

I've been using std::mutex lately and is now looking for a pattern/design guide on deleting an object that has std::mutex as member. The problem arises when an object as a public function that uses the mutex(monitor function, critical section, etc) and when this object is deleted, it might have threads still waiting on the mutex. std::mutex have undefined behavior deleted while being locked by other thread, and so the problem arises.

I want to know what is a generally acceptable pattern to use in this case. Or if this is considered a bad coding style, a way to avoid this design, ie) do not delete an object whose mutex methods are still being waited on.

Example:

//a public method that uses mutex.
IAsyncAction^ XInputBase::flushTask()
{
    return create_async([this](){
        _monitorMutex.lock();
        if (_readyToFlush && !_noMoreFlush) {
            //should flush only once, block additional flush signals.
            _noMoreFlush = true;
            _monitorMutex.unlock();
            //actually flush
            concurrency::task<void> UITask = concurrency::create_task(Windows::ApplicationModel::Core::CoreApplication::MainView->CoreWindow->Dispatcher->RunAsync(Windows::UI::Core::CoreDispatcherPriority::Normal,
                ref new Windows::UI::Core::DispatchedHandler([=]()
            {
                onFlush();
            })));
        }
        else {
            _needToFlush = true;
            _monitorMutex.unlock();
        }        
    });
}

Tried solutions: Destructor waiting for mutex to completely unlock after all waiting threads are processed. I've implemented it by setting a flag outside of mutex, so that all threads in the method are either exiting or waiting on the mutex. Then I've locked the std::mutex for last time at destructor. Given that the std::mutex is fair, destructor should be unlocked last after other waiting threads are processed, then destroy the object.

This doesn't work because std::mutex does not guarantee fairness in most platforms.

Sort of working solution: Implement the object as a ref class or other reference counted object(smart pointers). Then implement the concurrent method as a lamda that holds a strong reference to the object. Once all other objects holding this object are deleted and all lamdas with mutexes are processed, this object will be deleted automatically.

I don't like this method as it poses some restriction on the rest of the code. For WinRT, this method does not give control which thread deletes this object, as a result, UI thread error can occur.

Answer 1

You cannot guard the lifetime of an object with data within the object.

You can guard an object with an external mutex.

So start with this:

template<class T>
struct locked_view {
  template<class F>
  auto operator->*(F&& f) const
  -> std::result_of_t< F(T const&) >
  {
    auto l = lock();
    return std::forward<F>(f)(*t);
  }
  locked_view(locked_view const&) = default;
  locked_view& operator=(locked_view const&) = default;
  locked_view()=default;
  explicit operator bool() const { return m&&t; }

  locked_view( std::mutex* min, T* tin ):m(min), t(tin) {}

private:
  std::unique_lock<std::mutex> lock() const {
    return std::unique_lock<std::mutex>(*m);
  }
  std::mutex* m;
  T* t;
};

template<class T>
struct locked {
  locked()=default;
  locked(locked&& o):
    t( o.move_from() )
  {}
  locked(locked const& o):
    t( o.copy_from() )
  {}
  locked& operator=(locked&& o) {
    auto tin = o.move_from();
    assign_to(std::move(tin));
    return *this;
  }
  locked& operator=(locked const& o) {
    auto tin = o.copy_from();
    assign_to(std::move(tin));
    return *this;
  }

  template<class U,
    std::enable_if_t<!std::is_same<std::decay_t<U>, locked>{}, int> =0
  >
  locked( U&& u ):
    t( std::forward<U>(u) )
  {}

  // stars of show:
  locked_view<T const> read() const
  {
    return {&m, std::addressof(t)};
  }
  locked_view<T> write()
  {
    return {&m, std::addressof(t)};
  }

  T move_from() {
    return write()->*[](T& tin){return std::move(tin);};
  }
  T copy_from() const {
    return read()->*[](T const& tin){return tin;};
  }
  template<class U>
  void assign_to( U&& u ) {
    write()->*[&](T& t){ t = std::forward<U>(u); };
  }
private:
  mutable std::mutex m;
  T t;
};

Use looks like:

locked<int> my_int = 7;

my_int.read()->*[](int x){ std::cout << x << '\n'; };

Next, stuff a std::unique_ptr<YourClass> into it. This will permit people to delete it.

Finally, share a std::shared_ptr<> to it.

So

template<class T>
using shared_locked = std::shared_ptr< locked< T > >;
template<class T>
using shared_locked_ptr = shared_locked< std::unique_ptr<T> >;

is your type.

To use, they do this:

shared_locked_ptr<widget> w;
w->read()->*[&]( auto& ptr ) {
  if (ptr) ptr->do_something();
};

where you get to check the lifetime within the locked block.

To thread-safe delete the object:

w->write()->*[&]( auto& ptr ) {
  ptr = {};
};

The lock is around the pointer to the widget, not around the widget. The pointer to the pointer to the widget is shared.

Code not tested, but similar design has worked before.