Search code examples
c++multithreadingatomicstdatomic

Deleting the container in atomic multi-threaded code


Consider the following code:

struct T { std::atomic<int> a = 2; };
T* t = new T();
// Thread 1
if(t->a.fetch_sub(1,std::memory_order_relaxed) == 1)
  delete t;
// Thread 2
if(t->a.fetch_sub(1,std::memory_order_relaxed) == 1)
  delete t;

We know exactly one of Thread 1 and Thread 2 will execute the delete. But are we safe? I mean suppose Thread 1 will execute the delete. Is it guaranteed that when Thread 1 started the delete, Thread 2 won't even read t?


Solution

  • note that call delete happens after Release in Thread 2 and Release in Thread 2 happens after Release in Thread 1

    so call delete in Thread 2 happens after Release in Thread 1 which not access t anymore after Release

    but in real life (not in this concrete example) in general we need use memory_order_acq_rel instead memory_order_relaxed.

    this is because the real objects usual have more data fields, not only atomic reference count.

    and threads can write/modify some data in object. from another side - inside destructor we need view all modifications made by other threads.

    because this every not last Release must have memory_order_release semantic. and last Release must have memory_order_acquire for view after this all modification . let some example

    #include <atomic>
    
    struct T { 
      std::atomic<int> a; 
      char* p;
    
      void Release() {
        if(a.fetch_sub(1,std::memory_order_acq_rel) == 1) delete this;
      }
    
      T()
      {
        a = 2, p = nullptr;
      }
    
      ~T()
      {
          if (p) delete [] p;
      }
    };
    
    // thread 1 execute
    void fn_1(T* t)
    {
      t->p = new char[16];
      t->Release();
    }
    
    // thread 2 execute
    void fn_2(T* t)
    {
      t->Release();
    }
    

    in destructor ~T() we must view result of t->p = new char[16]; even if destructor will be called in thread 2. if use memory_order_relaxed formal this is not guaranteed. but with memory_order_acq_rel

    thread after final Release , which will be executed with memory_order_acquire semantic too (because memory_order_acq_rel include it) will be view result of t->p = new char[16]; operation because it happens before another atomic operation on the same a variable with memory_order_release semantic (because memory_order_acq_rel include it)


    because still exist doubt, i try make yet bit another prove

    given:

    struct T { 
        std::atomic<int> a;
    
        T(int N) : a(N) {}
    
        void Release() {
            if (a.fetch_sub(1,std::memory_order_relaxed) == 1) delete this;
        }
    };
    
    • let a initialized to N (=1,2,...∞)
    • let Release() called exactly N time

    question: are code will be correct and T will be deleted ?

    let N = 1 - so a == 1 at begin and Release() called one time.

    here exist question ? somebody say that this is "UB" ? (a accessed after delete this begin execute or how ?!)

    delete this can not begin execute until a.fetch_sub(1,std::memory_order_relaxed) will be calculated, because delete this depended from result of a.fetch_sub. compiler or cpu can not reorder delete this before a.fetch_sub(1,std::memory_order_relaxed) finished.

    because a == 1 - a.fetch_sub(1,std::memory_order_relaxed) return 1, 1 == 1 so delete this will be called.

    and all access to object before delete this begin execute.

    so code correct and T deleted in case N == 1.

    let now in case N == n all correct. so look for case N = n + 1. (n = 1,2..∞)

    • a.fetch_sub is modifications of atomic variable.
    • All modifications to any particular atomic variable occur in a total order that is specific to this one atomic variable.
    • so we can say that some a.fetch_sub will be executed first (in order of modification a)
    • this first (in order of modification a) a.fetch_sub return n + 1 != 1 (n = 1..∞) - so Release() in which will be executed this first a.fetch_sub, exit without call delete this
    • and delete this yet not called - it will be called only after a.fetch_sub which return 1, but this a.fetch_sub will be called after first a.fetch_sub
    • and will be a == n after first a.fetch_sub finished (this will be before all other n a.fetch_sub)
    • so one Release (where first a.fetch_sub executed ) exit without delete this and it finish access object before delete this start
    • we now have n rest Release() calls and a == n before any a.fetch_sub, but this case already OK

    one more note for those who think that code not safe / UB.

    not safe can be only if we begin delete before any access of object finished.

    but delete will be only after a.fetch_sub return 1.

    this mean that another a.fetch_sub already modify a

    because a.fetch_sub is atomic - if we view it side effect (modification of a) - a.fetch_sub - no more access a

    really if operation write value to memory location (a) and after this access this memory again - this already not atomic by sense.

    so if we view result of atomic modification - it already completed and no more access variable

    as result delete will be already after all access to a complete.

    and here not need any special memory order (relaxed,acq,rel) for atomic. even relaxed order is ok. we need only atomicity of operation.

    memory_order_acq_rel need if object T containing not only a counter. and we want in destructor view all memory modifications to another fields of T