What's the problem when deconstructor does garbage collection ??

Question

In my class, I have a vector allocated on the heap, and therefore I need to delete it after a while. But doing that by the deconstructor gives me unexpected result. While when I do garbage collection by an explicit function (like one I made, garbage_collector()), it works fine. WHY ? And how can I fix it so it can be done by deconstructor? (Check out my code between lines 171 to 187)

https://code.sololearn.com/cd7JIdLQ2eZt/?ref=app

Answer 1

Ali_combination
Orin Cook got it right. The problem is that on line 50, 3 things happen:
1. instance_1 & instance_2 is evaluated, which returns a BooleanAlgebra object
2. That object is passed to the copy-assign operator of the 'result' object
3. There, the object is copied into 'result' and then it goes out of scope (gets deleted)

In the last step, as you can see, the object that gets deleted frees the pointer. This is made clear by adding some printing some information
https://code.sololearn.com/cC0vddD2y3m1/?ref=app
Clearly, the 'vector_t' object that gets deleted is the same that is being printed. But the 'BooleanAlgebra' objeft that gets deleted is also the same as 'eval' (in operator&), but different from the one being printed

This signfies the importance of custom copy/move constructors for classes that handle heap memory directly

(continued in another answer)

Answer 2

Ali_combination
Raw pointers aren't always bad, they have their place. References can't be used in many cases and smart pointers have some performance overhead (which usually doesn't matter). In the end, it depends on your use case. The decision can be very confusing at first, but with time you'll be able to decide what should be used where and how.

Answer 3

I was mostly just commenting out everything one by one in main to see if I could narrow the problem down a little, but also .display was crowding out the error messages lol

What I mean about pointers is: it looks like "matrix" is defined as a pointer to a vector, rather than a vector itself, so is it possible you're not getting deep copies but just passing around/copying pointers to the same (two) underlying vectors? It would explain why the destructor causes problems, if so

Answer 4

Ali_combination exercising with raw pointers is also good, actually, also smart ones bring their own problems, but they're worth learning because of their broad usage
if you want to learn about them i'd recommend this website: learncpp.com
that's one of the best c++ resources i've ever encountered

Answer 5

Ali_combination
[1/1]
By replacing your manually allocated matrix with a vector, you have removed all the memory bugs you had before. But yes, the code is much worse in terms of performance because now you're passing the vector by value and hence it is copied when you pass it to the constructor. As I said before, references are the answer. This code is nearly as fast as your original one, but simpler and without bugs
https://code.sololearn.com/cMQ1tJvp6qVD/?ref=app

I have made 2 changes to your latest code:
1. Added constructors for passing the vector by reference
2. Replaced BooleanAlgebra& with 'const BooleanAlgebra&' in all operator overloads, to allow both lvalues and rvalues as the right-hand-side of expression (this is insignificant, and I did this because I had initially misunderstood your problem with references)

Continued in another answer

Answer 6

The destructor is also called every time you finish performing an overridden operation, when eval goes out of scope.

Answer 7

[2/2]
I'm surprised that out of all the errors in your code, this is the one that causes a problem.

**COPYING POINTERS DOES NOT COPY THEIR CONTENTS**
On line 24, you are allocating memory into 'matrix', but on line 60, you overwrite it with another pointer. This way, you are not only leaking memory, but also taking some totally unknown pointer from the user.

In main, you are passing the constructor pointers to data that is located on the stack, and later you are freeing it yourself!. This is very very bad. On a normal day, you would notice the effects of this very soon. But here, all the disaster thankfully happens at the end when everything is done.

This is a strong case of unncessarily passing around pointers. You shoud instead take a reference and copy the vector into your own matrix. Better yet, just use a vector as your matrix. You also need to create copy/move constructors to deal with the error i descibed in my previohs answer

Answer 8

try debugging the code in you IDE to see its execution order
also you can use references or smart pointers for efficiency and non-manual memory management

Answer 9

Ali_combination
[2/2]
As for point 2 of your "my problems have a long story" answer, I don't think I understand completely, but I believe that what you in mind was that the BooleanAlgebra class would not *own* any vector, i.e. it would just take a pointer to it and perform operations without making a new vector.

That is not a bad idea. But at that point, it's better to get rid of your class, and just have all the functionality in public operator overloads (or names functions) that take a reference to a vector_t, for example
vector_t operator ^ (vector_t const&, vector_t const&);
vector_t operator ^ (vector_t const&, int);

Or, if you do want to have a class that takes a pointer to a vector, then *do not free the pointer yourself*. Leave it to the user, C libraries do that all time. Do not tamper woth the memory that you take. Just take a pointer (const in your case), do something with it, and leave the destruvtor of your class empty.

Answer 10

XXX thanks, however I need to learn more. There are things in your code that I'm not familiar with.

I finally debugged my first program and saw that, the segmentation fault error was raised because:
For example in line 107, I'm trying to allocate some space for eval.matrix (whose type is a ptr), while it's NOT even initialized! A pointer needs to point to something, if we want to work with it. Otherwise we'll face dangerous situations :

vector<int>* ptr;
ptr->resize(10);

This simply raises 'segmentation fault' :)

Answer 11

XXX
I also learned a new thing here :
People discourage me from using raw pointers, as you did too. I better use smart ones, as programmers do. C++ can be less challenging by using smart ptrs (but I'm still not familiar with them).

Unfortunately currently i got no access to my program, so I explain it to you:
I rewrote my program like this, using raw pointers:

private:
vector_t* arg;
vector_t res;

here, arg is initialized with the passed matrix, and the result of the multiplication of two given matrices are stored in 'res'. Using 'res' eliminates the the segmentation fault.
Of course pass by ref is better than pass by ptr, I couldn't do that, because compiler complained :

In BooleanAlgebra()
std::vector<std::vector<bool>, std::allocater<bool>>& matrix should be initialized, reference data must be initialized at its definition.

In BooleanAlgebra(int)
std::vector<std::vector<bool>, std::allocater<bool>> should be initialized ...

However you did it successfully !! 🤔 i didn't know it's possible !

Answer 12

The code is all honestly a bit over my head so my confidence here is close to zero, but the way it's breaking seems familiar, and if you comment out .display() it says double free detected, so:
Are you perhaps just passing around pointers to the same matrix in all of these? That would result in the one and only underlying matrix getting deleted the first time you called any of your functions, when eval goes out of scope, which probably would cause this kind of problem

Answer 13

Orin Cook what do you mean by passing around pointers to the same matrix ?
In the code above, we have two distinct matrices. One operation (no matter which one) is done between the two. The result is stored in another object, and this object is returned.
Why do you comment out .display () ? Because I'm using this method in the main function... display only shows the result.

also when I return the result as a pointer and try to delete it by the deconstructor, it says : free() invalid pointer.

Answer 14

Orin Cook I'm a bit confused. The deconstructor is I think called when the program reaches the end of the main function (when it reaches return 0;) and at this time matrix is deleted, while the display () is called before reaching the end of the main function ! 🤔so before deleting the matrix, the content of the vector is "displayed" and AFTER THAT it is deleted. When calling display(), the content of the vector still exists ! it's gonna be deleted later ... So logically there shouldn't be any problem, should be ? 🤔 Im really like a deer caught in the headlights.

Answer 15

Orin Cook if the deconstructor is called when eval goes out of scope, then it makes sense.

Answer 16

XXX my problems have a long story ...

1- I strongly want to pass by ptr (and preferably ref, if possible) just for the sake of efficiency.
2- I failed to pass by ref, because my operator overloading can be done in two different ways :
BooleanAlgebra instance1(vec), instance2(3);
BooleanAlgebra result = instance1 ^ instance2;

AND

BooleanAlgebra instance1(vec1), instance2(vec2);
BooleanAlgebra result = instance1 & instance2;

So based on what kind of operation I am doing, there may be different combinations of objetcs (a vector with an integer, and a vector with a vector). If I pass by reference, the compiler will complain : 'std::vector<vector<bool>>& matrix;' must be initialized ...
So I finally lost my hope for passing by ref.
3- I failed to pass by ptr (while allocating my matrix vector on the stack) because of this code :
https://code.sololearn.com/c4FcgMnmzpKK/?ref=app
The compiler says : segmentation fault.

After all this long story, I RESORTED to allocating the matrix vector on the heap ...

Answer 17

XXX at first I thought to myself I finally managed it ... But looks like from the perspective of professional programmers, this one is sadly a mess again ... Now I don't know what to do.

Answer 18

XXX is this code a better one ?
https://code.sololearn.com/cGp9ZbR1I6PA/?ref=app

However, I wanted my code to be as efficient as possible (like the way they work with Qt Widgets, almost everything is done by pointers for the sake of efficiency). Is there any way to do so, but without making my code look like "a strong case of unnecessarily passing around pointers" ?

Answer 19

Patrick you are right. Using raw pointers could be challenging under some circumstances. If I already learned smart pointers, probably I wouldn't be dealing with dangling pointers, segmentation fault error, memory violation etc ...