Deleting std::vector Array in C++

HPBSM
HPBSM used Ask the Experts™
on
Hi,

I have the following below, is this code legal? can this code can cause to some heap corruption?
The main issue here if there is a double deletion of the vector objects.
The desctructor of class dd_sample_t checks if the members are not null, if there not null it deletes them and then puts a null in them.

I'll happy to provide any other data needed.

Thanks.

At the beggining:

{
      auto_ptr<CWtSampleCtor> sample_ctor(sample_constructor);

      // construct the samples
      vector<dd_sample_t *> samples_arr;

      sample_ctor->fillSamplesArr(samples_arr);

      //send the samples, do some actions....

      sample_ctor->freeSamplesArr(samples_arr);

}

The fuction fillSamplesArr(vector<dd_sample_t *> &samples_arr):

{
...
   
m_trc_general_sample.fill(...);

m_trc_general_sample.fill(...);

...

// The definition of m_trc_general_sample is:
// dd_sample_t      m_trc_general_sample;

samples_arr.push_back(&m_trc_general_sample);

}

The function freeSamplesArr(vector<dd_sample_t *> &samples_arr):
{
      // note - avoid deleting the last m_trc_general_sample sample

      for (size_t i = 0; i < samples_arr.size() -1; ++i)
      {
            if(samples_arr/[i/] != NULL)
            {
                  delete samples_arr/[i/];
                  samples_arr/[i/] = NULL;
            }
      }
}
Comment
Watch Question

Do more with

Expert Office
EXPERT OFFICE® is a registered trademark of EXPERTS EXCHANGE®
Top Expert 2009

Commented:
>> samples_arr.push_back(&m_trc_general_sample);

How is m_trc_general_sample defined ? This looks like the object has not been allocated using new, so you should not use delete to de-allocate it !!


Also : your freeSamplesArr function will try to delete all pointers in the vector, and then set them to NULL, but the size of the vector will still be the same. is that intentional ?

Author

Commented:
As I've written, m_trc_general_sample is defined as: dd_sample_t      m_trc_general_sample;

Indeed, it was not allocated by new but the details were created by create function (appears in definition.h attached)

Actually fillSamplesArr(vector<dd_sample_t *> &samples_arr) is defined as follows:


{

m_trc_general_sample.create(...); // defined in definition.h attached
...
   
m_trc_general_sample.fill(...); // defined in definition.h attached

m_trc_general_sample.fill(...); // defined in definition.h attached

...


samples_arr.push_back(&m_trc_general_sample); // std::vector function.

}

The class dd_sample_t definition is attached.

The fact is that from time to time this process hangs up and stops working, it is not consistent and can happen only after few days of running.

1. Can this code can cause to some heap corruption which can explain the facts?

2. Do you have any recommendation how to get close to the core of the problem? This process contians a huge amount of code and I've just suspected this area.

3. Is there any need to remove the code of freeSamplesArr?

Thanks a lot!

Author

Commented:
I'm attaching class dd_sample_t  definition.
definition.h
Introduction to Web Design

Develop a strong foundation and understanding of web design by learning HTML, CSS, and additional tools to help you develop your own website.

Author

Commented:
Regarding your remark:

"Also : your freeSamplesArr function will try to delete all pointers in the vector, and then set them to NULL, but the size of the vector will still be the same. is that intentional ?"

What are the implications of leaving the vector size the same?
Top Expert 2009
Commented:
>> Indeed, it was not allocated by new

Then you shouldn't use delete on it either.

There's another possible problem : if the life span of m_trc_general_sample is shorter than that of the vector, you will end up with a dangling pointer in the vector, with all bad consequences that brings.

>> 1. Can this code can cause to some heap corruption which can explain the facts?

Yes :
(a) calling delete for a pointer that wasn't returned by new will very likely corrupt the heap.
(b) dereferencing a dangling pointer, and using the object it's pointing to, is also likely to corrupt the heap.


>> 2. Do you have any recommendation how to get close to the core of the problem? This process contians a huge amount of code and I've just suspected this area.

If you need to track down a heap corruption, the debugger is your friend. Watch the memory area where the corruption occurs, and then step through the code, and see what happens.


>> 3. Is there any need to remove the code of freeSamplesArr?

Since it seems to call delete's for pointers that weren't returned by a new call, I'd say it's best to remove the delete's yes.
Top Expert 2009

Commented:
>> What are the implications of leaving the vector size the same?

That you haven't actually removed anything from the vector. Depending on how the vector is used in the rest of the code, this might or might not be an issue.

Author

Commented:
(b) dereferencing a dangling pointer, and using the object it's pointing to, is also likely to corrupt the heap.

1. Can you please explain what does it mean dereferencing  and a dangling pointer?

2. m_trc_general_sample is a class member and the vector is local in the function. Do you think that the life span of m_trc_general_sample is shorter than that of the vector?

3. In a debugger, the process was hang up as usual and no exception was thrown (all C++ exceptions were marked) I've breaked in Visual C++ 2005 and checked the call stacks of the threds but there was no connection to the vector. If the process is hang up and not crashes, how the debugger can help me?

Thanks
Top Expert 2009
Commented:
>> 1. Can you please explain what does it mean dereferencing  and a dangling pointer?

Dereference means to follow the pointer to the object it's pointing to (syntax : *ptr).
A dangling pointer is a pointer that no longer points to valid memory (the memory it used to point to, has no longer been allocated for the object in question).


>> 2. m_trc_general_sample is a class member and the vector is local in the function. Do you think that the life span of m_trc_general_sample is shorter than that of the vector?

If m_trc_general_sample is a non-static class member, then its life span is the same as that of the instantiated object. Does the object exist for the entire life span of the vector ?


>> 3. In a debugger, the process was hang up as usual

You'll have to set a breakpoint before the issue occurs, and then step through the code until it occurs. Observe the memory, variable values, call flows, etc. while doing that.

Author

Commented:
O.K. Thanks, I'll check it out and write again.
Top Expert 2009

Commented:
Let me know if you have further questions about this :)
evilrixSenior Software Engineer (Avast)

Commented:
This question has been classified as abandoned and is being closed as part of the Cleanup Program.  See my comment at the end of the question for more details.

Do more with

Expert Office
Submit tech questions to Ask the Experts™ at any time to receive solutions, advice, and new ideas from leading industry professionals.

Start 7-Day Free Trial