Dealing with memory issues caused by global static variables

Question

Warning. This question arose because I had to work with a huge pile of awful code without proper documentation written by someone else as a research project 6 years ago. Obviously, the best solution should not cause these problems in the first place, with proper design ...

This suggests that the question is: what is the best way to get out of this situation:

• The class allocates memory on the heap and frees it in the destructor.
• Somewhere, an instance of the class is declared in the global scope.
• There is a function that initializes this instance.
• The return value of this function is used to initialize the static variable.
• A global scope variable is used outside the static scope.

Minimum working example:

File "myclass.h":

#ifndef MYCLASS_H #define MYCLASS_H #include<vector> using namespace std; class myclass{ vector<int> *onTheHeap; public: myclass(int len=0){ onTheHeap = new vector<int>(len); } ~myclass(){ delete onTheHeap; } }; #endif 

file "static_loader.cpp"

 #include "class.h" myclass existsForever; int cause_static_global_creation(){ existsForever = myclass(5); } static int bootstrap = cause_static_global_creation(); 

and file "main.cpp":

 #include "class.h" extern myclass existsForever; int main(){ return 0; } 

Build with:

 g++ -g -c static_loader.cpp g++ -g main.cpp static_loader.o 

and run as:

  valgrind --leak-check=full ./a.out 

Result: the variable is freed when its destructor is called in the output handler below the main one, but also in the static_initialization_and_destruction_0 function below main from static_loader!

Is there a way to ensure that these variables are released exactly once, which is not related to the need to re-encode the code? In the library I have to work with, there are several dozen copies of this template ...

EDIT:

Adding Functions:

  void operator=(myclass other){ delete this->onTheHeap; this->onTheHeap = other.onTheHeap; } 

and

  myclass(const myclass& other){ this->onTheHeap = new vector<int>(*(other.onTheHeap)); } 

Does not change behavior.

Second EDIT:

  myclass& operator=(const myclass& other){ delete this->onTheHeap; this->onTheHeap = new vector<int>(*(other.onTheHeap)); return *this; } 

Solves all problems. My library has a memory leak with such sources anyway, but I'm not sure how to reproduce it. At least this is not so, and thanks for the refactoring suggestions, etc.!

Answer 1

Your assumption is broken. myclass existsForever; not cause_static_global_creation initialized, but myclass::myclass . Instead, cause_static_global_creation assigns a value to an already initialized object.

And since the class violates the rule of three, it is not surprising that the task causes problems.

Answer 2

myclass is created twice in your example.

First, with len=0 statement myclass existsForever; .

Later, a temporary instance is created in cause_static_global_creation with len=5 and assigned using the default assignment operator existsForever . Currently existsForever and the temporary value will have the same pointer value onTheHeap . Temporary is destroyed immediately in cause_static_global_creation , freeing up memory for the vector. The same memory is freed again at the end of the program when the global instance is destroyed.

I have some tips on how to fix this quickly.

1. Define the constructor as follows:

 myclass(int len=0) { if ( len > 0 ) onTheHeap = new vector<int>(len); else onTheHeap = NULL; } 

2.Use smart pointer instead of bare.

 std::shared_ptr<vector <int>> onTheHeap; 

3. Do not create a vector on the heap; use an instance instead.

The longer way is to correctly implement the assignment operator and copy constructor.

Answer 3

I think these comments / strategies cover your situation:

• If you own the .h class, just replace vector<int>* with vector . The compiler takes care of managing the stack memory and you avoid heap leaks.
• Please note that global statics will consume stack space for program execution time, therefore byte-for-byte is as bad as dynamic memory leak.
• Your double deletion is probably caused by a pointer escaping reference, since, according to Benji, the default copy is ctor. You could replace this with a generic pointer (which doesn't need to be deleted), but the stack is better. Consider disabling the copy constructor or writing a ctor copy that contains deep copies instead of small copies, and, as Benji said, if you disable copy destination and build and don't compile, you found (one of) your problem (s).
• Global statics should not be a problem when using memory, unless they are a collection and expand unlimitedly without removing their garbage after use. If the vector inside does not expand indefinitely without being cleared, then they consume a constant amount of memory in relation to the execution of the program. Providing them with a faster lifespan will lead to better code, but will be considered premature optimization if your problem is memory.