double free detected in tcache 2 when calling erase() on std::set

Question

i was writing a simple code for testing purposes. if i declare the node1 first, there's no problem but when i declare the node2 first, the code give me free(): double free detected in tcache2.

could someone please explain the reason for this double free error?

class node1;
class node2;

class node1{
 public:
     node1(void) = default;

     // member data
     int *n;
     std::set<node2*> tonode2;
};

class node2{
 public:
     node2(void) = default;

     ~node2(void){
         for(auto val:tonode1)
             val->tonode2.erase(this);           // free(): double free detected here..
     }

     // member data
     int *n;
     std::set<node1*> tonode1;
};

int main(void){
   node2 n2;
   node1 n1;

   n1.tonode2.insert(&n2);
   n2.tonode1.insert(&n1);

   return 0;
 }

the code compiled with g++ (GCC) 11.2.0 run on ubuntu 20.04.

`n1` is destroyed before `n2`, which still holds a reference. — Ulrich Eckhardt, Mar 16 '22 at 09:55
Actually double deletion is just the visible effect of invoking undefined behaviour, which occurred earlier. — Aconcagua, Mar 16 '22 at 10:24
You might fix by letting `node1` remove it's references from `node2` as well, then `n2` gets destructed its set is cleared already. However, you rely on *always* mutually including nodes in their sets, so you should make the sets private and provide a function that does such mutual inclusion as the *only* way to do so. — Aconcagua, Mar 16 '22 at 10:36

score 2 · Answer 1 · answered Mar 16 '22 at 09:57

2

n1 is destroyed first. When n2 is destroyed, its destructor will indirect through the pointer that used to point to n1, but is now invalid. The behaviour of the program is undefined.

answered Mar 16 '22 at 09:57

eerorika

232,697
12
197
326

Özgür Murat Sağdıçoğlu · Answer 2 · 2022-03-16T10:16:10.050

You better use smart pointers such as std::shared_ptr instead of raw pointers when ownership is important. std::weak_ptr should be preferred if there are reference cycles as in your question.

And you should create them with std::make_shared in the main, so that objects will be owned by smart pointer. Because smart pointers assume they are the only owner of the enclosed object. Otherwise it will be destructed two times, once by the owner scope and once by the smart pointer, leading to a double free corruption.

double free detected in tcache 2 when calling erase() on std::set

2 Answers2