Synchronization in destructor: Why not? How?

Question

Or... how to properly combine Concurrency, RAII and Polymorphism?

This is a very practical issue. We were bitten by this combination, summarized as Chandler Carruth's terrifying bug (at 1:18:45 mark)!

If you like bugs, try to catch the puzzle here (adapted from Chandler's talk):

#include <condition_variable>
#include <iostream>
#include <mutex>
#include <thread>

class A {
 public:
  virtual void F() = 0;
  void Done() {
    std::lock_guard<std::mutex> l{m};
    is_done = true;
    cv.notify_one();
    std::cout << "Called Done..." << std::endl;
  }
  virtual ~A() {
    std::unique_lock<std::mutex> l{m};
    std::cout << "Waiting for Done..." << std::endl;
    cv.wait(l, [&] {return is_done;});
    std::cout << "Destroying object..." << std::endl;
  }

 private:
  std::mutex m;
  std::condition_variable cv;
  bool is_done{false};
};

class B: public A {
 public:
  virtual void F() {}
  ~B() {}
};

int main() {
  A *obj{new B{}};

  std::thread t1{[=] {
    obj->F();
    obj->Done();
  }};

  delete obj;
  t1.join();

  return 0;
}

The issue (spotted when compiled via clang++ -fsanatize=thread) boils down to a race between a read of the virtual table (polymorphism) and a write on it (before entering ~A). The write being done as part of the destruction chain (so no method from B is called in A's destructor).

The recommended workaround is to move the synchronization outside the destructor, forcing each client of the class to call a WaitUntilDone/Join method. This is easy to forget and that's exactly why we wanted to use the RAII idiom in the first place.

Thus, my questions are:

Is there a nice way to enforce synchronization in base destructor?
Out of curiosity, why on earth the virtual table is even used from the destructor? I would have expected static binding here.

You just need to `join` your thread before you destroy `obj`. The destructor should not be performing synchronization. If you want to apply RAI you could put your shared object in a `std::shared_ptr` and synchronize on a result object or worker object instead. Edit : In this case though, there isn't really shared ownership. The issue is `A::~A` is synchronizing instead of `join`. — François Andrieux, Dec 10 '18 at 19:56
If you are concerned about exceptions and are using `std::thread` you have to remember to catch and somehow communicate exceptions in the worker thread. Consider using `std::async` and `std::future` instead, as it takes care of that concern for you. — François Andrieux, Dec 10 '18 at 19:57
I'm not sure which features you are referring to in *"another instance of C++ features shooting in each other's feet"*. — François Andrieux, Dec 10 '18 at 20:01
As a general rule, you will often get a better response if you refrain from negativity (`C++ features shooting in each other's feet`) — SergeyA, Dec 10 '18 at 20:03
See [this question](https://stackoverflow.com/questions/12092933/calling-virtual-function-from-destructor) for information on why the v table may need to change during destruction (and construction). During destruction, `obj` first ceases to be a `B` after `B::~B` and then ceases to be a `A` after `A::~A`. During `A::~A` `obj` acts as if it's dynamic type was `A` despite being created as a `B`. — François Andrieux, Dec 10 '18 at 20:05
Perhaps the best way to illustrate the problem with synchronizing in the destructor is to consider what happens if `obj` is destroyed `Done` executes. You're calling a non-`static` member function on a destroyed object. — François Andrieux, Dec 10 '18 at 20:08
Thank you François and Sergey. 1/ The aim is precisely to avoid having to call join() or other synchronization mechanism, in the same way smart pointers remove the 'delete' burden and RAII idiom in general makes sure things are cleaned/closed. 2/ The link doesn't answer the question: why dynamic patching inside constructor/constructor instead of early binding? — YvesgereY, Dec 17 '18 at 15:38

score 0 · Answer 1 · answered Dec 17 '18 at 17:00

You can use an RAII wrapper for objects that can't support RAII semantics directly. The wrapper object's constructor can construct the inner object and then do anything that's logically part of construction but can only be done after the inner object's constructor returns. Similarly, the wrapper object's destructor can do anything that has to be done before the inner object's destructor can safely be invoked and then it can destroy the inner object.

Synchronization in destructor: Why not? How?

1 Answers1