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

Question

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 1

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.

Answer 2

My answer is not related to the particular context in the question but related to the title. Here I got

free(): double free detected in tcache 2
Aborted (core dumped)

The above error messages when executing my binary with the main() { } method. When checking with memory leak tool nothing detected except that every string static member for classes used this this program is marked as leaked, which is impossible. So commented out all code in main. Still got the problem. After careful look into the make file, found the problem was due to linking. For a subset of C++ files they are linked twice. One through the temporary lib, another time through the make dependency. It took me two days to figure this out. Hope this posting can help the few who is not careful enough.

Here is the code fragment of my Makefile.am file for illustration of my problem.

lib_LTLIBRARIES = libfoo.la
libfoo_la_SOURCES=/*many not listed*/ abc.cpp xyz.cpp
LDADD = libfoo.la /*many others*/
prog_SOURCES=prog.cpp abc.cpp
prog_LDADD = $(LDADD) -lpthread /*some others*/

Removing abc.cpp from prog_SOURCES solved the problem

Answer 3

This may be part of an exercise to manage your own memory, but in production I try to never ever ever call new or delete.

I'd do this:

class Foo {
public:
    Foo() = default;
    Foo(int the_val, std::string the_str)
        : val{the_val}, str{std::move(the_str)}
    {
        std::cout << *val << '\n';
        std::cout << *str << '\n';
    }

    ~Foo() {
        if (!val.has_value()) {
            std::cout << "val is empty\n";
        }

        if (!str.has_value()) {
            std::cout << "str is empty\n";
        }
    }

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

    Foo(Foo&& rhs) = default;

private:
    std::optional<int> val;
    std::optional<std::string> str;
};

or if you don't really need the optionalness:

class Foo1 {
public:
    Foo1() = default;
    Foo1(int the_val, std::string the_str)
        : val{the_val}, str{std::move(the_str)}
    {}

private:
    int val = 0;
    std::string str;
};

https://godbolt.org/z/1WPPex33h