Home > Blockchain >  Why does my UniquePtr implementation double free?
Why does my UniquePtr implementation double free?

Time:07-02

When I run this program I get a double free for my implementation of unique ptr. Any idea why this happens?

#include <iostream>
#include <memory>
using namespace std;

template <class T>
class UniquePtr
{
public:
        UniquePtr(T* t = nullptr) : t_(t) {}
        UniquePtr(const UniquePtr&) = delete;
        UniquePtr& operator=(const UniquePtr& oth) = delete;
        UniquePtr(UniquePtr&& oth) {
                std::swap(t_, oth.t_);
        }
        UniquePtr& operator=(UniquePtr&& oth) {
                std::swap(t_, oth.t_);
                return *this;
        };
        ~UniquePtr() {
                delete t_;
        }

private:
        T* t_;
};

struct Obj {
        Obj(int x): x_(x) { cout << "new " << x_ << endl; }
        ~Obj() { cout << "delete " << x_ << endl; }
        int x_;
};


template <class UP>
void test(UP&&) {
        {
                UP ptr(new Obj(1));
        }
        {
                UP ptr(UP(new Obj(2)));
        }
        {
                auto lambda = []() {
                        UP ptr(new Obj(3));
                        return ptr;
                };
                UP ptr2(lambda());
        }
}
int main() {
        cout << "unique_ptr" << endl;
        test(unique_ptr<Obj>());
        cout << endl;

        cout << "UniquePtr" << endl;
        test(UniquePtr<Obj>());
        cout << endl;

        return 0;
}
unique_ptr
new 1
delete 1
new 2
delete 2
new 3
delete 3

UniquePtr
new 1
delete 1
new 2
delete 2
new 3
delete 3
delete 0
free(): double free detected in tcache 2
Aborted

CodePudding user response:

t_ is uninitialised in your move constructor so the moved from pointer ends up pointing to an uninitialised pointer and has undefined behaviour (this will cause problems when the moved from object destructs and deletes the uninitialised pointer). You need:

UniquePtr(UniquePtr&& oth)
: t_(nullptr)
{
    std::swap(t_, oth.t_);
}

Or possibly simpler:

UniquePtr(UniquePtr&& oth)
: t_(std::exchange(oth.t_, nullptr)
{
}
  • Related