In C++, when reassigning an object, why does the constructor fire before the destructor?

Question

I'm a beginner in C++. I'm compiling my code with GCC 9.2.0. Recently I've stumbled upon some weird behaviour which is segfaulting me.

I have a certain class wrapper for Hyperscan. I've made it so it basically has the same functionality as Python's re, but many times faster and multithreaded. I have a class regex which I usually allocate like

auto some_regex = regex();

Now, the what the constructor does, for those of you unfamiliar with Hyperscan is 2 (actually 3) things -> first it allocated memory for the regular expression database. This is done by calling their API, which is in C. Then, if it succeeds, you allocate scratch space. Now, because my implementation supports multithreading, my class instance holds a member which is a vector of scratch spaces. Nevertheless, I allocated a scratch space for each thread I use, also with their API. What I then have are a pointer to the compiled regex database and a vector of scratch space pointers.

When deconstructing the object, I have to free the database and scratch spaces (I also use the provided functions from the API) if they're allocated. So far so good. The problem arises when I do something like this:

auto some_regex = regex();
some_regex = regex(R"(\s+)");

Do you see a problem here? Maybe I'm too much of a noob to see it, but it looks good to me. But nope. It segfaults in the destructor while trying to free the database memory. Debugging it, I have come across these actions:

allocate memory for some_regex
initialize some_regex
allocated new memory for some_regex
initialize new some_regex
run destructor for the old some_regex
run destructor for the new some_regex

Basically, the destructor for the first instance is run only after the constructor for the second one fires. What this does is essentially making it so that the first destructor frees up the data allocated and initialized by the second constructor instead of just cleaning itself.

Now, things get weirder. I looked into the addresses at hand, because I started setting the freed up pointers to nullptr - it turns out that this happens:

allocate memory for some_regex
initialize some_regex
allocated new memory for some_regex
initialize new some_regex
run destructor for the old some_regex
database pointer is not 0, free it
set database pointer to nullptr
run destructor for the new some_regex
database pointer is not 0, free it (BUT THIS IS ALREADY FREED MEMORY???)
SIGSEGV

I'm utterly confused. Even after setting the database pointer to null explicitly, the new one, after the constructor did its job, assigns it a new address.

This reproduces the behaviour:

#include <iostream>
#include <malloc.h>

mock::mock()
{
    std::cout << "Starting constructor..." << std::endl;
    this->test = (int*)malloc(sizeof(int));
    *this->test = 0;
    std::cout << "Ending constructor..." << std::endl;
}

mock::mock(int x)
{
    std::cout << "Starting full constructor..." << std::endl;
    this->test = (int*)malloc(sizeof(int));
    *this->test = x;
    std::cout << "Ending full constructor..." << std::endl;
}

mock::~mock()
{
    std::cout << "Starting destructor...";
    free(this->test);
    this->test = nullptr;
    std::cout << "Address of test is " << this->test << std::endl;
    std::cout << "Ending destructor..." << std::endl;
}

run it in main like this:

auto some_mock = mock();
some_mock = mock();
some_mock = mock();
some_mock = mock(3);

Expected result:

Starting constructor...
Ending constructor...
Starting destructor...Address of test is 0
Ending destructor...
Starting constructor...
Ending constructor...
Starting destructor...Address of test is 0
Ending destructor...
Starting constructor...
Ending constructor...
Starting destructor...Address of test is 0
Ending destructor...
Starting full constructor...
Ending full constructor...
Starting destructor...Address of test is 0
Ending destructor...
Process returned 0

Actual result:

Starting constructor...
Ending constructor...
Starting constructor...
Ending constructor...
Starting destructor...Address of test is 0
Ending destructor...
Starting constructor...
Ending constructor...
Starting destructor...Address of test is 0
Ending destructor...
Starting full constructor...
Ending full constructor...
Starting destructor...Address of test is 0
Ending destructor...
*** Error in `/deploy/cmake-build-local-debug/CPP': double free or corruption (fasttop): 0x0000000002027c40 ***
.
.
.
Starting destructor...
Process finished with exit code 134 (interrupted by signal 6: SIGABRT)

Is this intended behaviour? If so, why??? And how can I get around it (aside from using new and deleting before reassigning)?

You are not following the rule of [three/five/zero](https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)). — Max Langhof, Sep 09 '19 at 16:45
Side-note: prefer `new` to `malloc` in C++ code. `malloc` is a C function and may not behave as expected due to the object model. — Deji, Sep 09 '19 at 16:48
@Deji "Prefer" and "may not behave as expected" is putting it _very_ lightly. If you don't manually call constructors/destructors with `malloc`/`free` (if you have to ask "should I do that" then you're not ready to do that) then you are right in Undefined Behavior land. — Max Langhof, Sep 09 '19 at 16:50
Yes, I expected as much, but didn't quite know how to put it, thanks. — Deji, Sep 09 '19 at 16:51
@Deji You see, the thing is that I didn't write Hyperscan - it's a library maintained by intel which has C API. For me to write it "correctly" I would have to rewrite it completely (every thing is C style), which kind of defeats the purpose of just using well tested and maintained software in the first place. — Ljac, Sep 09 '19 at 20:46

Deji · Answer 1 · 2019-09-09T16:52:22.940

1

You're missing a key element here: assignment operators.

Given

auto some_mock = mock();
some_mock = mock();

In the first initialization, the compiler is smart enough to know to construct some_mock directly (the mock() temporary is elided).

On the second line it's not initialization, but rather assignment, the destructor of some_mock will not be called. Instead, it will be reassigned. A temporary rvalue object is constructed with mock(), then that temporary is assigned to some_mock, then the temporary is destructed.

What you want to do if you're handling resources is follow the rule of All or Nothing.

edited Sep 09 '19 at 16:52

answered Sep 09 '19 at 16:46

Deji

760
1
10
16

What do yu mean with "directly" in _"construct `some_mock` directly"_ ? – Paul Ogilvie Sep 09 '19 at 16:50
1

@PaulOgilvie Presumably the elision of the temporary (which has become "mandatory" - as in, the temporary no longer conceptually exists, the entire thing is just initialization - with C++17). – Max Langhof Sep 09 '19 at 16:51
So what you're saying is that the problem lies in the fact that I didn't explicitly define the assign operator? I mean, I get that I should implement all 5, sorry that this is duplicated, I just really never imagined it's called something like this, I searched the net and SO for this but I guess the question was asked in too stupid of a way and there was really no one who works with C++ where I work at that I could talk to about this. – Ljac Sep 09 '19 at 20:53
@Ljac Well the problem just appears to be a misunderstanding of how assignments work. They don't just destruct and re-construct, they execute a whole different method. This is beneficial because an object may not necessarily have to fully reset before being re-assigned. I assume you're just missing that in your test because you're not explicitly defining assignment operators. – Deji Sep 09 '19 at 21:05
@Deji I tested it a bit right now and that seemed to be a problem. Will definitely look into it tomorrow and fix my implementation to work correctly. – Ljac Sep 09 '19 at 21:15

In C++, when reassigning an object, why does the constructor fire before the destructor?

1 Answers1