C raw pointers using this for operator overload results in segmentation fault when freeing memory-CodePudding

I am trying to build a Tree of operations for a Tensor application in c . When I write c = a b I would like c to have as children two pointers a and b. I pass the "this" of a to the constructor of c and then I free the memory in the destructor.

template<typename T>
struct ObjectPointers {
    const ObjectPointers<T> *left;
    const ObjectPointers<T> *right;
    vector<T> data;

    // initialize left and right in constructor
    ObjectPointers(std::initializer_list<T> list) : data(list) {
        left = nullptr;
        right = nullptr;
    }

    ~ObjectPointers(){
        // somewhere around here the error happens
        if(left != nullptr)
            delete left;
        if(right != nullptr)
            delete right;
    }

    ObjectPointers(const ObjectPointers<T> *left, const ObjectPointers<T> *right) : left(left), right(right) {}

    //overload  
    ObjectPointers operator (const ObjectPointers &other) const {
        // I create a new object with the left and right of the current object
        return ObjectPointers<T>(this, &other);
    }
};

int main() {
    ObjectPointers<int> a = {1, 2, 3};
    ObjectPointers<int> b = {4, 5, 6};
    ObjectPointers<int> c = a   b;

    return 0;
}

The way I understand the code that I wrote is as follows: Object c is created and points to a and b.

c goes out of scope => It calls the destructor => delete a => destructor of a is called => Nothing happens

=> delete b => Destructor of b is called => Nothing happens => Destructor of c is done

Where I wrote "Nothing happens" in reality a segmentation fault happens and I do not understand why.

I also tried using smart_pointers but that did not really help. I used std::unique_ptr<>.

CodePudding user response：

What's the problem?

This is not a sound design, because it does not respect the usual properties of , for example that (x y) z is the same than x (y z).

If you nevertheless want to make it work, you'd need to extract the operator of the class, and use an overload of the binary operator .

In addition, you have to work properly with destruction, and avoid destroying an object which is still used in some of the ObjectPointers. For this you should use shared_ptr and not unique_ptr.

(The advantage is that when an object gets destroyed, the shared pointer is kept alive as long as another object still uses the pointer. Your current design with raw pointers might lead multiple ObjectPointers use the same pointer, and the first ObjectPointers that gets deleted, deletes the pointer, and the others then refer to dangling pointers. This is undefined behavior and very bad)

How to solve it?

The code would look somewhat like this (caution, I didn't analyse if it further for other issues):

template<typename T>
struct ObjectPointers {
    const std::shared_ptr<ObjectPointers<T>> left;
    const std::shared_ptr<ObjectPointers<T>> right;
    std::vector<T> data;

    // initialize left and right in constructor
    ObjectPointers(std::initializer_list<T> list) : data(list) {
    }

    ObjectPointers(const ObjectPointers &c) = default;
    ~ObjectPointers(){  // no need,  shared ptr take care
     }

    ObjectPointers(std::shared_ptr<ObjectPointers<T>> left, std::shared_ptr<ObjectPointers<T>> right) : left(left), right(right) {}

};

And:

//overload  
template <class T>
ObjectPointers<T> operator  (ObjectPointers<T> a, ObjectPointers<T> b)  {
     // I create a new object with the left and right of the current object
     return ObjectPointers<T>(std::make_shared<ObjectPointers<T>>(a), std::make_shared<ObjectPointers<T>>(b));
}

Here an online demo, including printing of the resulting structure.