Problem overloading operator new

[mahlerfive]

My goal here is to write a small class to keep track of memory I allocate/deallocate so that I can more easily find memory leaks. To do this I overloaded the operators new, new[], delete, and delete[]. In those methods, I allocate/free memory as usual, but also make a call to my MemoryTracker object which stores this information. The MemoryTracker is a singleton.

My problem is that whenever I call new, it seems to get called infinitely many times. This only occurs if the memorytracker is currently up and running (i.e. instantiated from main). If I run this code, once it hits the new char line, it seems to call the new operator over and over with size 24 - no idea why. If anyone sees what I'm doing wrong it would be greatly appreciated!

EDIT: I just realized that in the Allocate() method, whenever I add a pair to the memory map, it is instantiating that pair object, thus the infinite recursion. Can anyone think of a good way to avoid this problem?

Here is my main:

#include <cstdlib>
#include <iostream>
#include "MemoryTracker.h"
#include "overloaded_new_delete.h"

using namespace std;

MemoryTracker* memory_tracker = 0;

int main(int argc, char *argv[]) {
    // Create the memory tracker
    memory_tracker = MemoryTracker::Instance();
    
    char* test0 = new char( 'a' );  
    memory_tracker->PrintStatus();
    memory_tracker->Destroy();
    
    system("PAUSE");
    return EXIT_SUCCESS;
}

Here are my overloaded operators (header file):

#ifndef __OVERLOAD_NEW_DELETE_H__
#define __OVERLOAD_NEW_DELETE_H__

#include <iostream>
#include <cstdlib>
#include "MemoryTracker.h"

using namespace std;

extern MemoryTracker* memory_tracker;

void* operator new( size_t size );
void* operator new[]( size_t size );
void operator delete( void *p );
void operator delete[]( void *p );

#endif 

Here are my overloaded operators (implementation):

#include "overloaded_new_delete.h"

void* operator new( size_t size ) {

    void *p = malloc( size );
    if (!p)
        throw "operator new() error";
    cout << p << endl;
    cout << size << endl;
    system("pause");
    
    if( memory_tracker != 0 ) {
        memory_tracker->Allocate( p, size );
    }

    return p;
}

void* operator new[]( size_t size ) {  
    void *p = malloc(size);
    if (!p)
        throw "operator new() error";
    
    if( memory_tracker != 0 ) {
        memory_tracker->Allocate( p, size );
    }
    
    return p;
}

void operator delete( void *p ) {  
    free(p);
    if( memory_tracker != 0 ) {
        memory_tracker->Free( p );
    }
}

void operator delete[]( void *p ) {
    free(p);
    if( memory_tracker != 0 ) {
        memory_tracker->Free( p );
    }
}

Here is my MemoryTracker class header file:

#ifndef __MEMORY_TRACKER_H__
#define __MEMORY_TRACKER_H__

#include <iostream>
#include <map>
#include <fstream>
using namespace std;

class MemoryTracker {
    
    public:
        static MemoryTracker* Instance();
        static MemoryTracker* Instance( const string& filename );
        void Destroy();
        void Allocate( const void* ptr, const size_t size );
        void Free( const void* ptr );
        void PrintStatus();
        

    protected:
        MemoryTracker();
        ~MemoryTracker();
        
        
    private:
        static MemoryTracker* _instance;
        map<const void *, const size_t> _memory_map;
        static string _OUTPUT_FILENAME;
        size_t _total_size;
        int _num_pointers;

};

#endif 

Here is my MemoryTracker class implementation:

#include "MemoryTracker.h"

// Initialize instance pointer
MemoryTracker* MemoryTracker::_instance = 0;

string MemoryTracker::_OUTPUT_FILENAME = "memorytracker.log";

/**
 * CONSTRUCTOR
**/
MemoryTracker::MemoryTracker() {
    _memory_map.clear();
    _num_pointers = 0;
    _total_size = 0;
}


/**
 * DESTRUCTOR
**/
MemoryTracker::~MemoryTracker() {
    _memory_map.clear();
    _num_pointers = 0;
    _total_size = 0;
}


/**
 * Destroys the instance.
**/
void MemoryTracker::Destroy() {
    delete _instance;
    _instance = 0;
}


/**
 * Returns the instance of this singleton.
**/
MemoryTracker* MemoryTracker::Instance() {
    if( _instance == 0 ) {
        _instance = new MemoryTracker;
    }
    
    return _instance;
}


/**
 * Call this when allocating memory with overloaded new or new[].
**/
void MemoryTracker::Allocate( const void* ptr, const size_t size ) {
    _memory_map.insert( pair<const void*, const size_t>( ptr, size ) );
    _total_size += size;
    _num_pointers++;
}


/**
 * Call this when freeing memory with delete or delete[].
**/
void MemoryTracker::Free( const void* ptr ) {
    size_t size = _memory_map[ ptr ];
    _memory_map.erase( ptr );
    _total_size -= size;
    _num_pointers--;
}


/**
 * Prints the status of all the allocated memory being tracked to file.
**/
void MemoryTracker::PrintStatus() {
    // Open the log file for output
    ofstream out( _OUTPUT_FILENAME.c_str() );
    
    // Print Totals
    out << "Total Number of Pointers: " << _num_pointers << endl;
    out << "Total Size Allocated: " << _total_size << endl;
    
    out << endl << endl;
    
    // Print specific info about each pointer
    out << "Pointer Information:" << endl;
    map< const void*, const size_t >::iterator it;
    for( it = _memory_map.begin(); it != _memory_map.end(); it++ ) {
        out << "Address: " << it->first << " Size: " << it->second << endl;
    }
    
    // Close log file
    out.close();
}

[jsmith]

This is very useful!

One way to solve your problem (which isn't necessarily easy, but...) is to write a custom allocator for map. Recall that the declaration of map has 4 template parameters:

template< typename _Key, typename _Tp, typename _Compare = less<_Key>,
   typename _Alloc = allocator<pair<const _Key, _Tp>>> class map {};

Your custom allocator would need to avoid using any form of new, which means it couldn't use any STL containers.

Unfortunately writing an allocator isn't easy, and I couldn't even begin to provide any help or suggestions off the top of my head (while my code is compiling here...)

[anilpanicker]

This coincide with my task in office this week that is memory leak fixing for a large program. If you are using Microsoft's Visual C++ IDEs you can use the <crtdbg.h> header. Here is a sample program (got it from the net) which I found very useful as a first cut.
After you run your program, it will list the leaks in the output

#include <vector>

// 1. this should go into every .cpp , after all header inclusions
#ifdef _WIN32
#ifdef _DEBUG
   #include <crtdbg.h>
   #undef THIS_FILE
   static char THIS_FILE[] = __FILE__;
   #define new       new( _NORMAL_BLOCK, __FILE__, __LINE__)
   #define malloc(s) _malloc_dbg(s, _NORMAL_BLOCK, __FILE__, __LINE__)
#endif
#endif

   void foo(){
	 // create some leaks
    std::vector<int*> lstAllocs;
    for (int x=0; x<99; x++)
    {
        int* pInt = new int;
        lstAllocs.push_back( pInt );
    }
   }
int main(int argc, char* argv[])
{
    // 2. at the beginning of our app we need this
    int tmpFlag = _CrtSetDbgFlag( _CRTDBG_REPORT_FLAG );
        tmpFlag |= _CRTDBG_LEAK_CHECK_DF;
    _CrtSetDbgFlag( tmpFlag );
//	_CrtSetBreakAlloc(169); //After you find the block number you want to do a break at line use this

   foo();
    return 0;
}

The output is given below
Detected memory leaks!
Dumping objects ->
C:\ANILL\L42\L42\main.cpp(19) : {169} normal block at 0x003248B8, 4 bytes long.
Data: < > CD CD CD CD
C:\ANILL\L42\L42\main.cpp(19) : {168} normal block at 0x00324870, 4 bytes long.
Data: < > CD CD CD CD
C:\ANILL\L42\L42\main.cpp(19) : {167} normal block at 0x00324828, 4 bytes long.
-----

[mahlerfive]

@jsmith: Yes, that does sound like a crazy amount of work just to fix this problem...

@anilpacker: Unfortunately I'm not using Visual C++ IDE and I would prefer a solution that does not require inserting code into every .cpp file in the project.

The only other solution I've thought of so far is to set a flag in my MemoryTracker just before I insert into the map to say "ignore tracking the next allocation". However, this would only work in a single-threaded environment and I would want something that works in a multi-threaded environment as well.

[Zaita]

I am going to be honest here. You are re-inventing the functionality of many tools already available, they do NOT require you to modify your code.

e.g AQTime. This tool is able to track memory leaks in your application, not just in the objects you have written (e.g I get a Win32 API handle, but don't free it). Plus many other things.

I would suggest using a tool like this instead of adding unnecessary overhead into your project.

[anilpanicker]

Zaita,
Can you please suggest ant free tool.

[mahlerfive]

@Zaita: I know there are memory management tools out there, however I do not know of any free ones. It looks like AQtime is not only not free, but only works with windows. I am also writing this to learn more about c++, however I would have no problem learning to use a free memory tracking tool if you know of one.

[jsmith]

One to try is valgrind (and is free):

http://directory.fsf.org/project/valgrind/

[watters]

Your problem is in MemoryLeak::Instance(). You are using the new operator in this method and it has already been overloaded, resulting in an infinite recursion. You would need to allocate the MemoryLeak object using malloc to bypass this problem.

[jsmith]

Um, not only is the thread more than a month old, but the author of the code came to that realization in the original post and my original reply already said to do that...