CODEWITHSUNDEEP
c++memory-leaksgdb
Schilcote
Schilcote

Reputation: 2394

Mysterious memory leak in C++ file output function

I hate to do the old "here's some code, what's wrong with it?" but I've been investigating this issue for several days with no progress. This code usually crashes immediately with the debugger off, though occasionally there'll be a space of a few hours where it'll compile and run correctly. With gdb attached it works but leaks memory at about 10 MB per second and eventually crashes when it runs out.

It's definitely this function; no such issues occur when the only call to it is commented out, and calling it less often delays the out-of-memory crash.

 //Write a frame of the animation to disk
void draw_frame(int framenum) {
    ofstream fout;
    ostringstream fname;
    fname << "D:\\frames\\" << framenum << ".data";
    fout.open(fname.str(), ios::binary | ios::out);

    unsigned char ***frame;
    frame = new unsigned char ** [WORLDSIZE];
    for (int x = 0; x < WORLDSIZE; x++) {
        frame[x] = new unsigned char * [WORLDSIZE];
        for (int y=0; y < WORLDSIZE; y++) {
            frame[x][y] = new unsigned char [3];
        }
    }

    unsigned long long ***colormix;
    colormix = new unsigned long long ** [WORLDSIZE];
    for (int x = 0; x < WORLDSIZE; x++) {
        colormix[x] = new unsigned long long * [WORLDSIZE];
        for (int y=0; y < WORLDSIZE; y++) {
            colormix[x][y] = new unsigned long long [3];
            for (int z=0; z < 3; z++) {
                colormix[x][y][z]=0;
            }
        }
    }

    for (vector<SmellyObject *>::iterator it = smellythings.begin(); it != smellythings.end(); ++it) {
        SmellyObject *theobj = *it;
        for (int x=0; x < WORLDSIZE; x++) {
            for (int y=0; y < WORLDSIZE; y++) {
                double scentlevel = scentmaps[theobj -> id][x][y];
                double colorlevel = (scentlevel / 10000.0);
                colormix[x][y][0] += theobj->r * colorlevel;
                colormix[x][y][1] += theobj->g * colorlevel;
                colormix[x][y][2] += theobj->b * colorlevel;
            }
        }
    }

    for (int x=0; x < WORLDSIZE; x++) {
        for (int y=0; y < WORLDSIZE; y++) {
            for (int z=0; z < 3; z++) {
                //cout << colormix[x][y][z] << " ";
                frame[x][y][z] = min(255.0, (colormix[x][y][z] / (double) smellythings.size()));
            }
        }
    }

    for (int x=0; x < WORLDSIZE; x++) {
        for (int y=0; y < WORLDSIZE; y++) {
            fout.write((char *) frame[x][y], 3);
        }
    }

    fout.close();

    frame = new unsigned char ** [WORLDSIZE];
    for (int x = 0; x < WORLDSIZE; x++) {
        frame[x] = new unsigned char * [WORLDSIZE];
        for (int y=0; y < WORLDSIZE; y++) {
            for (int z=0; z < WORLDSIZE; z++) {
                //delete[] &frame[x][y][z];
            }
            delete[] frame[x][y];
        }
        delete[] frame[x];
    }
    delete[] frame;

    colormix = new unsigned long long ** [WORLDSIZE];
    for (int x = 0; x < WORLDSIZE; x++) {
        colormix[x] = new unsigned long long * [WORLDSIZE];
        for (int y=0; y < WORLDSIZE; y++) {
            for (int z=0; z < WORLDSIZE; z++) {
                //delete[] &colormix[x][y][z];
            }
            delete[](colormix[x][y]);
        }
        delete[](colormix[x]);
    }
    delete[](colormix);

    return;
}

WORLDSIZE is 50, and scentmaps is a std::unordered_map mapping ints (the .id property of a SmellyObject) to WORLDSIZE x WORLDSIZE arrays of doubles.

Upvotes: 1

Views: 372

Answers (1)

Sam Varshavchik
Sam Varshavchik

Reputation: 118415

frame = new unsigned char ** [WORLDSIZE];

You allocate this array at the beginning of the function, and proceed to allocate all of its three dimensions, gobbling up a huge amount of memory.

Then, later...

fout.close();

frame = new unsigned char ** [WORLDSIZE];

... you just reassigned frame to another allocated array. The old frame pointer is gone, and you've just leaked the metric ton of memory you allocated initially.

Same bug with your other, colormix array.

Lesson to be learned from this: the best way to fix bugs is to never make them in the first place. Had you been using C++ containers that properly handle all memory allocations for you, like std::vector, this would've never happened. Correctly using the full resources of the C++ library -- containers, iterators, algorithms -- make it logically impossible for many common programming bugs to happen.

Upvotes: 1

Related Questions