free(): double free detected in tcache 2 in C++

eminomur picture eminomur · Sep 21, 2019 · Viewed 16k times · Source

Firstly, I really checked if there is a question already been asked but I could not find any. Error message should not deceive you my situation is a bit different I guess or I am just missing something.

While I was dealing with a toy C++ code I encountered with a weird error. Program output says there is double free situation but I cannot see the place where this error occurs. Code may be seen a bit long, I apology for that.

I am working on a Linux Distribution right now and I am using g++ 9.1.0. I checked my code and looked for erroneous part.

Even though I fixed some part of code, my problem did not get solved except when I comment either Foo{1, "Hello World"}; or vec.push_back(std::move(Foo{})); and I don't get it why.

class Foo
{
public:
    Foo()
        : val{nullptr}, str{nullptr}
    {
        std::cout << "You are in empty constructor\n";
    }

    Foo(int the_val, const char *the_str)
        : val{new int}, str{new char[std::strlen(the_str + 1)]}
    {
        *val = the_val;
        std::cout << *val << '\n';
        std::strcpy(str, the_str);
        std::cout << str << '\n';
    }

    ~Foo()
    {
        if (val) {
            delete val;
        } else {
            std::cout << "val is empty\n";
        }

        if (str) {
            delete[] str;
        } else {
            std::cout << "str is empty\n";
        }
    }

    Foo(const Foo&) = delete;
    Foo& operator= (const Foo&) = delete;

    Foo(Foo&& rhs)
    {
        std::cout << "Move constructor is triggered\n";

        if (val) {
            delete val;
        }
        val = rhs.val;
        rhs.val = nullptr;

        if (str) {
            delete[] str;
        }
        str = rhs.str;
        rhs.str = nullptr;
    }

    Foo& operator= (Foo& rhs)
    {
        std::cout << "Move assignment is triggered\n";

        // Self-assignment detection
        if (&rhs == this) {
            return *this;
        }

        if (val) {
            delete val;
        }
        val = rhs.val;
        rhs.val = nullptr;

        if (str) {
            delete[] str;
        }
        str = rhs.str;
        rhs.str = nullptr;

        return *this;
    }
private:
    int *val;
    char *str;
};


int main()
{
    Foo{1, "Hello World"};

    std::vector<Foo> vec;
    vec.push_back(std::move(Foo{}));

    return 0;
}

If I do not comment any place in function main, output is as follows.

1
Hello World
You are in empty constructor
val is empty
str is empty
You are in empty constructor
Move constructor is triggered
free(): double free detected in tcache 2
Aborted (core dumped)

If I comment "Foo{1, "Hello World"};", output becomes

You are in empty constructor
Move constructor is triggered
val is empty
str is empty
val is empty
str is empty

and finally, when I comment "vec.push_back(std::move(Foo{}));", output becomes

You are in empty constructor
Move constructor is triggered
val is empty
str is empty
val is empty
str is empty

Answer

Vlad from Moscow picture Vlad from Moscow · Sep 21, 2019

For starter this constructor uses a wrong mem-initializer

Foo(int the_val, const char *the_str)
    : val{new int}, str{new char[std::strlen(the_str + 1)]}
                                             ^^^^^^^^^^^

I think you mean

Foo(int the_val, const char *the_str)
    : val{new int}, str{new char[std::strlen(the_str  ) + 1]}

This move constructor is also invalid

Foo(Foo&& rhs)
{
    std::cout << "Move constructor is triggered\n";

    if (val) {
        delete val;
    }
    val = rhs.val;
    rhs.val = nullptr;

    if (str) {
        delete[] str;
    }
    str = rhs.str;
    rhs.str = nullptr;
}

Within the body of the constructor the data members val and str have indeterminate values. They were not initialized when the body of the constructor got the control.

You could write it the following way

Foo(Foo&& rhs) : val( nullptr ), str( nullptr )
{
    std::cout << "Move constructor is triggered\n";

    std::swap( val, rhs.val );
    std::swap( str, rhs.str );
}

This operator

Foo& operator= (Foo& rhs)

is not a move-assignment operator. It is a copy assignment operator. So its definition is incorrect.

Also this statement in main

Foo{1, "Hello World"};

does not make sense. The object is created and at once is deleted.

In this statement

vec.push_back(std::move(Foo{}));

std::move is redundant because Foo{} is already an rvalue.