Is it correct to run code inside a method whose object has been destroyed?

Consider an object for which a method is invoked. Beyond certain point the method no longer accesses this at all. No read/writes of non-static members. No invocation of non-static methods. Is it safe to destroy such object in another thread after being sure the method has gone beyond such point even if it has not finished executing the whole method?

Consider the following example:

#include <mutex>
#include <condition_variable>
#include <memory>
#include <thread>

class InstrumentedMutex {
public:
  void lock() {
    internalMutex.lock();
  }
  void unlock() {
    internalMutex.unlock();
    printf("unlocked\n"); //A. Maybe undefined behaviour.
    variable = 3;         //B. Undefined behaviour for sure
  }                       //C. Maybe undefined behaviour.
  std::mutex& mutex() {
    return internalMutex;
  }
private:
  std::mutex internalMutex;
  int variable;
};

std::unique_ptr<InstrumentedMutex> instMutex;
std::condition_variable cv;
bool flag = false;

void f1() {
  instMutex->lock();
  flag = true; 
  cv.notify_one();
  instMutex->unlock();
}

int main() {
  instMutex.reset ( new InstrumentedMutex() );
  std::thread extra_thread(f1);
  std::unique_lock<std::mutex> lock(instMutex->mutex());
  cv.wait(lock, []() { return flag; });
  lock.unlock();
  lock.release();
  instMutex.reset(); // Will this cause undefined behaviour
                     // regarding comments A, B and C?
  extra_thread.join();
  return 0;
}

Line with comment B is undefined behaviour for sure because of this possible execution schedule:

     main thread               extra_thread
     -----------               ------------
     cv.wait(lock) // unlocks
     // lock and waits 
                               instMutex->lock();
                               flag=true
                               cv.notify_one()
                               instMutex->unlock() {
                                 internalMutex.unlock(); 
                                 // At this point the cv has been notified
                                 // the flag set and the mutex
                                 // unlocked. So main thread may
                                 // continue

    lock.unlock();
    lock.release();
    instMutex.reset(); 
    // At this point the instMutex
    // object has been destroyed
                               // But here we are executing
                               // "inside" the destroyed object.
                               printf("unlocked\n");
                               variable = 3; // Here we are 
                               // writing to freed memory.

Would line for comment A also be undefined behaviour?

What about line for comment C? Which is meant to be when the method has executed completely (and any destructors for local objects which might reference this have also finished executing), and the method just has to return.

"Has just to return" is not very formal and kind of assumes a stack based implementation. So maybe a better way to express the question regarding line C would be: If a method has no code after a synchronization barrier (not even destructors for local objects), is it always safe to destroy the associated object after ensuring that such synchronization barrier has completed?

posted almost 4 years ago

CC BY-SA 4.0

Estela‭

599 reputation 10 8 90 22