c++ memory leak caused by vector of structs

Question

The memory leak caused by the line indicated. "pendingSendReqs.push_back(&f);" in the sendreq() method. I am new to c++ so I can't seem to figure out why the memory leak is occuring. The size of memory leaked is 16 bytes.

class Station {
    struct Frame {
        enum { Token, Data, Ack } type;                  // type of frame
        unsigned int src;                                // source id
        unsigned int dst;                                // destination id
        unsigned int prio;                               // priority
    } frame;

    unsigned int stnId;
    static unsigned int requests;                        // total send requests (if needed)
    void data( Frame frame );                            // pass frame
    void main();                                         // coroutine main
    Station *nextStation;
    vector<Frame*> pendingSendReqs;
  public:
    Station( unsigned int id ) : stnId(id) { }
    ~Station() {
        for (int i = 0; i < pendingSendReqs.size(); i++) {
            delete pendingSendReqs.at(i);
            cout << "~: " << pendingSendReqs.at(i) << endl;
        }
    }
    //unsigned int getId() { return stnId; }
    void setup( Station *nexthop ) {                     // supply next hop
        //*nexthop is the object
        nextStation = nexthop;
        //cout << "size: " << sizeof(*nexthop) << endl;
    }
    void sendreq( unsigned int round, unsigned int dst, unsigned int prio ) { // store send request
        Frame f;
        f.type = Frame::Data;
        f.src = stnId;
        f.dst = dst;
        f.prio = prio;


        pendingSendReqs.push_back(&f); //MEMORY LEAK CAUSED BY THIS LINE
    }
    void start();                                        // inject token and start
};

Just use a `vector`. No need for pointers. As is, this is a concern: http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope — chris, Jan 27 '14 at 21:55

score 5 · Answer 1 · answered Jan 27 '14 at 21:55

5

This is not a memory leak

pendingSendReqs.push_back(&f);

it is future undefined behaviour. You are storing the address of a local variable. Any attempt to de-reference one of those pointers outside of the scope of the function is undefined behaviour.

You have to ask yourself whether you really need a vector of pointers. If you don't know the answer to that, it is likely that you don't.

answered Jan 27 '14 at 21:55

juanchopanza

223,364
34
402
480

So what's the right way to store this struct in this case? – Nathan B Apr 27 '20 at 11:56

legends2k · Accepted Answer · 2014-01-27T22:06:43.763

You're storing pointers to local variables, which will automatically get destroyed, inside the vector. This is illegal.

 vector<Frame*> pendingSendReqs;
 // this is a vector of pointers to struct and not a vector of structs

void sendreq( unsigned int round, unsigned int dst, unsigned int prio ) {
    Frame f;     // this automatic variable will get destroyed when sendreq returns
    f.type = Frame::Data;
    f.src = stnId;
    f.dst = dst;
    f.prio = prio;


    pendingSendReqs.push_back(&f); //MEMORY LEAK CAUSED BY THIS LINE
    // because you're going to hold on to this address which will mean
    // nothing when this function returns
}

The way you intend to do it is:

vector<Frame> pendingSendReqs;

and inside sendreq:

pendingSendReqs.push_back(f); // store the object's copy instead of it's address so that it outlives the life of this local

Did you remove `delete pendingSendReqs.at(i);` after switching to storing objects instead of pointers? The vector's destructor will automatically destroy them for you, so you don't've to manually call `delete` on what you didn't allocate using `new`. In fact, since it's the only thing you're doing inside `~Station()` you can get rid of the destructor altogether. — legends2k, Jan 27 '14 at 22:27

Jefferson Rondan · Answer 3 · 2014-01-27T23:17:04.973

2

when

void sendreq( unsigned int round, unsigned int dst, unsigned int prio )

ends,

your vector pendingSendReqs will contain pointers to variables that have been eliminated ( because are local variable ), and will contain garbage, and will give you a crash.

edited Jan 27 '14 at 23:17

answered Jan 27 '14 at 22:14

Jefferson Rondan

103
1
10

c++ memory leak caused by vector of structs

3 Answers3