Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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?

like image 819
Koosha Avatar asked Dec 30 '19 22:12

Koosha


2 Answers

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

like image 125
RbMm Avatar answered Nov 20 '22 03:11

RbMm


This should be safe assuming each thread only runs once because t wouldn't be deleted until both threads have already read the pointer. Although I would still strongly recommend the use of a std::shared_ptr if you want to manage the lifetime of a pointer with reference counting instead of trying to do it yourself. That's what it was made for.

suppose Thread 1 will execute the delete. Is it guaranteed that when Thread 1 started the delete, Thread 2 won't even read t?

Yes, in order for thread 1 to delete t, the read in the second thread that decrements the value must have already occurred otherwise the if statement would not have evaluated to true and t would not have been deleted.

like image 40
tjwrona1992 Avatar answered Nov 20 '22 03:11

tjwrona1992