Destructor, when object's dynamic variable is locked by mutex will not free it?

Question

I'm trying to solve some complicated (for me at least) asynchronous scenario at once, but I think it will be better to understand more simple case.

Consider an object, that has allocated memory, carrying by variable:

#include <thread>
#include <mutex>
using namespace std;

mutex mu;

class Object
{
public:
  char *var;

  Object()
  {
      var = new char[1]; var[0] = 1;
  }
  ~Object()
  {
      mu.lock();
      delete[]var; // destructor should free all dynamic memory on it's own, as I remember
      mu.unlock();
  }
}*object = nullptr;

int main()
{
   object = new Object();

   return 0;
}

What if while, it's var variable in detached, i.e. asynchronous thread, will be used, in another thread this object will be deleted?

void do_something()
{
    for(;;)
    {
         mu.lock();
         
         if(object)
             if(object->var[0] < 255)
                  object->var[0]++;
             else
                  object->var[0] = 0;

         mu.unlock();
    }
}

int main()
{
   object = new Object();

   thread th(do_something);
   th.detach();

   Sleep(1000);

   delete object;
   object = nullptr;

   return 0;
}

1. Is is it possible that var will not be deleted in destructor?
2. Do I use mutex with detached threads correctly in code above?

2.1 Do I need cover by mutex::lock and mutex::unlock also delete object line?

I also once again separately point that I need new thread to be asynchronous. I do not need the main thread to be hanged, while new is running. I need two threads at once.

---

P.S. From a list of commentaries and answers one of most important thing I finally understood - mutex. The biggest mistake I thought is that already locked mutex skips the code between lock and unlock.

Forget about shared variables, mutex itself has noting to do with it. Mutex is just a mechanism for safely pause threads:

mutex mu;

void a()
{
    mu.lock();
    Sleep(1000);
    mu.unlock();
}

int main()
{
    thread th(a);
    th.detach();

    mu.lock(); // hangs here, until mu.unlock from a() will be called
    mu.unlock();

    return;
}

The concept is extremely simple - mutex object (imagine) has flag isLocked, when (any) thread calls lock method and isLocked is false, it just sets isLocked to true. But if isLocked is true already, mutex somehow on low-level hangs thread that called lock until isLocked will not become false. You can find part of source code of lock method scrolling down this page. Instead of mutex, probably just a bool variable could be used, but it will cause undefined behaviour.

Why is it referred to shared stuff? Because using same variable (memory) simultaneously from multiple threads makes undefined behaviour, so one thread, reaching some variable that currently can be used by another - should wait, until another will finish working with it, that's why mutex is used here.

Why accessing mutex itself from different threads does not make undefined behaviour? I don't know, going to google it.

Answer 1

All you assumed and asked in your question is right. The variable will always be freed.

But your code has one big problem. Lets look at your example:

int main()
{
   object = new Object();

   thread th(do_something);
   th.detach();

   Sleep(1000);

   delete object;
   object = nullptr;

   return 0;
}

You create a thread that will call do_something(). But lets just assume that right after the thread creation the kernel interrupts the thread and does something else, like updating the stackoverflow tab in your web browser with this answer. So do_something() isn't called yet and won't be for a while since we all know how slow browsers are.

Meanwhile the main function sleeps 1 second and then calls delete object;. That calls Object::~Object(), which acquires the mutex and deletes the var and releases the mutex and finally frees the object.

Now assume that right at this point the kernel interrupts the main thread and schedules the other thread. object still has the address of the object that was deleted. So your other thread will acquire the mutex, object is not nullptr so it accesses it and BOOM.

PS: object isn't atomic so calling object = nullptr in main() will also race with if (object).

Answer 2

Do I use mutex with detached threads correctly in code above?

Those are orthogonal concepts. I don't think mutex is used correctly since you only have one thread mutating and accessing the global variable, and you use the mutex to synchronize waits and exits. You should join the thread instead.

Also, detached threads are usually a code smell. There should be a way to wait all threads to finish before exiting the main function.

Do I need cover by mutex::lock and mutex::unlock also delete object line?

No since the destructor will call mu.lock() so you're fine here.

Is is it possible that var will not be deleted in destructor?

No, it will make you main thread to wait though. There are solutions to do this without using a mutex though.

There's usually two way to attack this problem. You can block the main thread until all other thread are done, or use shared ownership so both the main and the thread own the object variable, and only free when all owner are gone.

To block all thread until everyone is done then do cleanup, you can use std::barrier from C++20:

void do_something(std::barrier<std::function<void()>>& sync_point)
{
    for(;;)
    {

         
         if(object)
             if(object->var[0] < 255)
                  object->var[0]++;
             else
                  object->var[0] = 0;


    } // break at a point so the thread exits
    sync_point.arrive_and_wait();
}

int main()
{
   object = new Object();

   auto const on_completion = []{ delete object; };

   // 2 is the number of threads. I'm counting the main thread since
   // you're using detached threads
   std::barrier<std::function<void()>> sync_point(2, on_completion);

   thread th(do_something, std::ref(sync_point));
   th.detach();

   Sleep(1000);
   sync_point.arrive_and_wait();

   return 0;
}

Live example

This will make all the threads (2 of them) wait until all thread gets to the sync point. Once that sync point is reached by all thread, it will run the on_completion function, which will delete the object once when no one needs it anymore.

The other solution would be to use a std::shared_ptr so anyone can own the pointer and free it only when no one is using it anymore. Note that you will need to remove the object global variable and replace it with a local variable to track the shared ownership:

void do_something(std::shared_ptr<Object> object)
{
    for(;;)
    {
         
         if(object)
             if(object->var[0] < 255)
                  object->var[0]++;
             else
                  object->var[0] = 0;

    }
}

int main()
{
   std::shared_ptr<Object> object = std::make_shared<Object>();

   // You need to pass it as parameter otherwise it won't be safe
   thread th(do_something, object);
   th.detach();

   Sleep(1000);

   // If the thread is done, this line will call delete
   // If the thread is not done, the thread will call delete
   // when its local `object` variable goes out of scope.
   object = nullptr;

   return 0;
}

Answer 3

1. Is is it possible that var will not be deleted in destructor?

With

~Object()
{
    mu.lock();
    delete[]var; // destructor should free all dynamic memory on it's own, as I remember
    mu.unlock();
}

You might have to wait that lock finish, but var would be deleted.

Except that your program exhibits undefined behaviour with non protected concurrent access to object. (delete object isn't protected, and you read it in your another thread), so everything can happen.

2. Do I use mutex with detached threads correctly in code above?

Detached or not is irrelevant.

And your usage of mutex is wrong/incomplete. which variable should your mutex be protecting?

It seems to be a mix between object and var. If it is var, you might reduce scope in do_something (lock only in if-block)

And it misses currently some protection to object.

2.1 Do I need cover by mutex::lock and mutex::unlock also delete object line?

Yes object need protection.

But you cannot use that same mutex for that. std::mutex doesn't allow to lock twice in same thread (a protected delete[]var; inside a protected delete object) (std::recursive_mutex allows that).

So we come back to the question which variable does the mutex protect?

if only object (which is enough in your sample), it would be something like:

#include <thread>
#include <mutex>
using namespace std;

mutex mu;

class Object
{
public:
  char *var;

  Object()
  {
      var = new char[1]; var[0] = 1;
  }
  ~Object()
  {
      delete[]var; // destructor should free all dynamic memory on it's own, as I remember
  }
}*object = nullptr;

void do_something()
{
    for(;;)
    {
         mu.lock();
         
         if(object)
             if(object->var[0] < 255)
                  object->var[0]++;
             else
                  object->var[0] = 0;

         mu.unlock();
    }
}

int main()
{
   object = new Object();

   thread th(do_something);
   th.detach();

   Sleep(1000);

   mu.lock(); // or const std::lock_guard<std::mutex> lock(mu); and get rid of unlock
   delete object;
   object = nullptr;
   mu.unlock();

   return 0;
}

Alternatively, as you don't have to share data between thread, you might do:

int main()
{
   Object object;

   thread th(do_something);

   Sleep(1000);

   th.join();
   return 0;
}

and get rid of all mutex

Answer 4

Have a look at this, it shows the use of scoped_lock, std::async and managment of lifecycles through scopes (demo here : https://onlinegdb.com/FDw9fG9rS)

#include <future>
#include <mutex>
#include <chrono>
#include <iostream>

// using namespace std; <== dont do this
// mutex mu; avoid global variables.

class Object
{
public:
    Object() :
        m_var{ 1 }
    {
    }

    ~Object()
    {
    }

    void do_something()
    {
        using namespace std::chrono_literals;

        for(std::size_t n = 0; n < 30; ++n)
        {
            // extra scope to reduce time of the lock
            {
                std::scoped_lock<std::mutex> lock{ m_mtx };
                m_var++;
                std::cout << ".";
            }

            std::this_thread::sleep_for(150ms);
        }
    }


private:
    std::mutex m_mtx;
    char m_var;
    
};

int main()
{
    Object object;

    // extra scope to manage lifecycle of future
    {
        // use a lambda function to start the member function of object
        auto future = std::async(std::launch::async, [&] {object.do_something(); });

        std::cout << "do something started\n";
        
        // destructor of future will synchronize with end of thread;
    }
    std::cout << "\n work done\n";

    // safe to go out of scope now and destroy the object
    return 0;
}