Memory Leak Issue

[linuxdev]

Hi,

I have the following requirement,

Data shall be received at runtime from network, this data has an unique ID/Key
associated with it, say 100 or 103

Many such data can come over a period of time and it can come for the
for the same Key or different Keys. This data has to be displayed in
a GUI and capacity to store a maximum of 50 such data for a given Key
has to be provided.

Request to delete all the data associated with a Key, comes asynchronously
at runtime, which should clear all the data held for that Key.

To implement this i have used QHash

QHash<int, QVector<TTrack> *> hash_for_track;
class TTrack
{
  public:
    TTrack();
 
    ~TTrack()
    {
 
    }
    int operator==(TTrack other);
 
    unsigned short track_number:12;
    unsigned short reserved:4;
 
    unsigned char  track_status;
    unsigned char track_status_ext;
    signed short x_comp;
    signed short y_comp;
    signed short height;
 
    unsigned short calculated_ground_speed;
    unsigned short calculated_heading;
    UINT16 flight_level:14;
    UINT16 flt_lvl_garbel:1;
    UINT16 flt_lvl_validity:1;
    unsigned short rho;
    unsigned short theta;
    UINT16 Mode1Code:5;
    UINT16 Mode1CodeMatch:1;
    UINT16 Mode1Garbel:1;
    UINT16 Mode1Validity:1;
    UINT16 Mode2Code:12;
    UINT16 Mode2Confidencel:1;
    UINT16 Mode2CodeMatch:1;
    UINT16 Mode2Garbel:1;
    UINT16 Mode2Validity:1;
    UINT16 Mode3ACode:12;
    UINT16 Mode3AConfidencel:1;
    UINT16 Mode3ACodeMatch:1;
    UINT16 Mode3AGarbel:1;
    UINT16 Mode3AValidity:1;
//Code match
/*
    This is to hold the Code match
    Info for the New Track
*/
    bool mode1_code_match;
    bool mode2_code_match;
    bool mode3_code_match;
    bool code_match;
};

QHash<int, QVector<TTrack> *> hash_for_track;
class TTrack
{
  public:
    TTrack();

    ~TTrack()
    {

    }
    int operator==(TTrack other);

    unsigned short track_number:12;
    unsigned short reserved:4;

    unsigned char  track_status;
    unsigned char track_status_ext;
    signed short x_comp;
    signed short y_comp;
    signed short height;

    unsigned short calculated_ground_speed;
    unsigned short calculated_heading;
    UINT16 flight_level:14;
    UINT16 flt_lvl_garbel:1;
    UINT16 flt_lvl_validity:1;
    unsigned short rho;
    unsigned short theta;
    UINT16 Mode1Code:5;
    UINT16 Mode1CodeMatch:1;
    UINT16 Mode1Garbel:1;
    UINT16 Mode1Validity:1;
    UINT16 Mode2Code:12;
    UINT16 Mode2Confidencel:1;
    UINT16 Mode2CodeMatch:1;
    UINT16 Mode2Garbel:1;
    UINT16 Mode2Validity:1;
    UINT16 Mode3ACode:12;
    UINT16 Mode3AConfidencel:1;
    UINT16 Mode3ACodeMatch:1;
    UINT16 Mode3AGarbel:1;
    UINT16 Mode3AValidity:1;
//Code match
/*
    This is to hold the Code match
    Info for the New Track
*/
    bool mode1_code_match;
    bool mode2_code_match;
    bool mode3_code_match;
    bool code_match;
};

To copy to clipboard, switch view to plain text mode 

And the functions are
a. "track_dbase(TTrack local_track)"
One to store all the data related to New Key/ Old Key
b. "On_TrackDeleteSignal(const S_TRACK_DELETE_MSG &resp) "
Delete all the data associated with a Key


Function Implementation:

void CircleWidget::track_dbase(TTrack local_track)
{
	QHash<int, QVector<TTrack> *>::const_iterator iter_for_track_hash;
 
	UINT16 key = 0;
	key = local_track.track_number;
 
	/*
	 * 16-Oct-2009
	 * Track number is only 12 bits
	 * so bit wise AND with this value
	 */
	key = key & 0x0FFF;
 
	if (hash_for_track.isEmpty() ==true)
	{
		/* no key exists in the hash so diretly insert the key and pointer to the track data
		 into the HASH
		 */
 
		m_vtrack_ptr = new QVector<TTrack>;
		if (m_vtrack_ptr ==NULL)
		{
		}
		else
		{
			m_vtrack_ptr->append(local_track);
			hash_for_track.insert(key, m_vtrack_ptr);
		}
	}
	else
	{
 
		if (trial_enabled == 0)
		{
			/*here we dont have store the trial details for all the tracks
			 only one element needs to be there in the vector of each
			 trackid
			 */
			number_of_points_in_vector = 1;
 
		} //end of if trial enabled
		else
		{
			/* check to see if the key exists in the hash_for_track table
			 */
			number_of_points_in_vector = ELEMENTS_IN_TRACK_DBASE;
		}
 
		if (hash_for_track.contains(key) == false)
		{
			/*
			 update the vector wtth the current track data
			 insert the (key,value) into the hash
			 */
 
			m_vtrack_ptr = new QVector<TTrack>;
			if (m_vtrack_ptr ==NULL)
			{
			}
			else
			{
				m_vtrack_ptr->append(local_track);
				hash_for_track.insert(key, m_vtrack_ptr);
			}
 
		} //end of if key is not present
		else
		{
 
			/*hash table already has the key specified retreive the correcsponding value and then
			 update the vector
			 */
 
			/*
			 here come and append into the vector
			 if the size is less than 50
 
			 if the size is equal to 50 then remove the element at index 0 and then apend
			 because at any instant i need only 50 elements in the vector.
			 */
			iter_for_track_hash = hash_for_track.find(key);
			QVector<TTrack>* m_vtrack_ptr_alreadyinhash =
					iter_for_track_hash.value();
			if (m_vtrack_ptr_alreadyinhash == NULL)
			{
			}
			else
			{
				if (m_vtrack_ptr_alreadyinhash->size()
						< number_of_points_in_vector)
				{
					m_vtrack_ptr_alreadyinhash->append(local_track);
				}
				else
				{
					m_vtrack_ptr_alreadyinhash->remove(0);
					m_vtrack_ptr_alreadyinhash->append(local_track);
				}
			}
		}
	}
}
 
void QCSurveillance::On_TrackDeleteSignal(const S_TRACK_DELETE_MSG &resp)
{
	int number_trackdel = -1;
 
	if (hm->m_gen.m_cwid->hash_for_track.isEmpty() == false)
	{
		//Find the key in the Hash Table if found then delete
		if (hm->m_gen.m_cwid->hash_for_track.contains(resp.track_id) == true)
		{
 
			number_trackdel
					= hm->m_gen.m_cwid->hash_for_track.remove(resp.track_id);
			if (number_trackdel==0)
				if (RDSDEBUG)
					qWarning("Delete Failed %d", resp.track_id);
				else
					;
 
		}
		else
		{
			;
		}
	}
	else
	{
		qDebug("Track Delete warning: No track in the database");
	}
}

void CircleWidget::track_dbase(TTrack local_track)
{
	QHash<int, QVector<TTrack> *>::const_iterator iter_for_track_hash;

	UINT16 key = 0;
	key = local_track.track_number;

	/*
	 * 16-Oct-2009
	 * Track number is only 12 bits
	 * so bit wise AND with this value
	 */
	key = key & 0x0FFF;

	if (hash_for_track.isEmpty() ==true)
	{
		/* no key exists in the hash so diretly insert the key and pointer to the track data
		 into the HASH
		 */

		m_vtrack_ptr = new QVector<TTrack>;
		if (m_vtrack_ptr ==NULL)
		{
		}
		else
		{
			m_vtrack_ptr->append(local_track);
			hash_for_track.insert(key, m_vtrack_ptr);
		}
	}
	else
	{

		if (trial_enabled == 0)
		{
			/*here we dont have store the trial details for all the tracks
			 only one element needs to be there in the vector of each
			 trackid
			 */
			number_of_points_in_vector = 1;

		} //end of if trial enabled
		else
		{
			/* check to see if the key exists in the hash_for_track table
			 */
			number_of_points_in_vector = ELEMENTS_IN_TRACK_DBASE;
		}

		if (hash_for_track.contains(key) == false)
		{
			/*
			 update the vector wtth the current track data
			 insert the (key,value) into the hash
			 */

			m_vtrack_ptr = new QVector<TTrack>;
			if (m_vtrack_ptr ==NULL)
			{
			}
			else
			{
				m_vtrack_ptr->append(local_track);
				hash_for_track.insert(key, m_vtrack_ptr);
			}

		} //end of if key is not present
		else
		{

			/*hash table already has the key specified retreive the correcsponding value and then
			 update the vector
			 */

			/*
			 here come and append into the vector
			 if the size is less than 50

			 if the size is equal to 50 then remove the element at index 0 and then apend
			 because at any instant i need only 50 elements in the vector.
			 */
			iter_for_track_hash = hash_for_track.find(key);
			QVector<TTrack>* m_vtrack_ptr_alreadyinhash =
					iter_for_track_hash.value();
			if (m_vtrack_ptr_alreadyinhash == NULL)
			{
			}
			else
			{
				if (m_vtrack_ptr_alreadyinhash->size()
						< number_of_points_in_vector)
				{
					m_vtrack_ptr_alreadyinhash->append(local_track);
				}
				else
				{
					m_vtrack_ptr_alreadyinhash->remove(0);
					m_vtrack_ptr_alreadyinhash->append(local_track);
				}
			}
		}
	}
}

void QCSurveillance::On_TrackDeleteSignal(const S_TRACK_DELETE_MSG &resp)
{
	int number_trackdel = -1;

	if (hm->m_gen.m_cwid->hash_for_track.isEmpty() == false)
	{
		//Find the key in the Hash Table if found then delete
		if (hm->m_gen.m_cwid->hash_for_track.contains(resp.track_id) == true)
		{

			number_trackdel
					= hm->m_gen.m_cwid->hash_for_track.remove(resp.track_id);
			if (number_trackdel==0)
				if (RDSDEBUG)
					qWarning("Delete Failed %d", resp.track_id);
				else
					;

		}
		else
		{
			;
		}
	}
	else
	{
		qDebug("Track Delete warning: No track in the database");
	}
}

To copy to clipboard, switch view to plain text mode 

Problem:
Application consumes memory continuosly, to debug the same i used Valgrind, which reported memory leak errors in the "new" operator used in the function "track_dbase".

I have removed the element in the hash using "remove" function for the given key in function "On_TrackDeleteSignal". Still i get memory leak error.

Query:
1. Is there any problem in the way i have used "new" operator in "track_dbase" function?
2.Does "remove" for a key in Hash remove all the memory explicitly used by value in the function "On_TrackDeleteSignal" ?
3. I have not defined the function "int operator==(TTrack other);" is it necessary to do it, and can this be a source of error?

I have limited time to solve this issue ... Kindly looking forward response from the community

[Lykurg]

Don't create QVector on the heap since it is implicitly shared. If you remove an item from the vector you also have to delete the item. This causes your Memory leak. You also might want to have a look at qDeleteAll().

[linuxdev]

Thanks for the reply

Meanwhile, i tried this code and it seems to work fine

QList<QVector<TTrack> *> trackidvectorlist =
							hash_for_track.values();
					/*
					 If all keys are selected then proceed to display
					 */
					for (int k = 0; k < trackidvectorlist.size(); k++) {
						if (NULL != trackidvectorlist.at(k))
							delete trackidvectorlist.at(k);
					}
 
					hash_for_track.clear();

QList<QVector<TTrack> *> trackidvectorlist =
							hash_for_track.values();
					/*
					 If all keys are selected then proceed to display
					 */
					for (int k = 0; k < trackidvectorlist.size(); k++) {
						if (NULL != trackidvectorlist.at(k))
							delete trackidvectorlist.at(k);
					}

					hash_for_track.clear();

To copy to clipboard, switch view to plain text mode 

Whenever i dont need an entry in Hash, i am getting the value, which is a pointer to Qvector in this case and deleting it explicitly.
Valgrind also doenst report any Memory Leak now.

Query:
1. Is the above mentioned code snippet safe to use?
2. I have done much of coding with QVector, how do i delete the item before removing it?
is it like, i have to call these two statements to do that?
delete vector.at(i) ;
vector.remove(i) ;
3. Can i use QList instead of QVector, as i need a Dynamic Array to store my elements, or is there any other better container for this problem?

[Lykurg]

Query:
1. Is the above mentioned code snippet safe to use?

Still, I would use the stack for the container classes... And the if in the for scope is not necessary, also use "++k".

2. I have done much of coding with QVector, how do i delete the item before removing it?
is it like, i have to call these two statements to do that?
delete vector.at(i) ;
vector.remove(i) ;

No, but whith a list it's much shorter.

delete list.takeAt(i);

delete list.takeAt(i);

To copy to clipboard, switch view to plain text mode 

3. Can i use QList instead of QVector, as i need a Dynamic Array to store my elements, or is there any other better container for this problem?

Depends on your needs. With QList you can do hardly something wrong. But there are also LIFO LILO containers. Yust choose the the one, which fits your accessing needs best.

[linuxdev]

Queries:

1.

delete vector.at(i) ;
 vector.remove(i) ;

delete vector.at(i) ;
 vector.remove(i) ;

To copy to clipboard, switch view to plain text mode 

Are these statements wrong ?
If so how do i delete an Item from QVector even after calling remove on that item?

2. Even if i use QList, i have to create a list of items in Heap , using the code
pointer = new QList<type>, (because of design constraints)
As i note from docs, even QList is implicitly shared, wont that create me the same problem? Or i still have to do?

delete list.takeAt(i);

delete list.takeAt(i);

To copy to clipboard, switch view to plain text mode 

3. Please do not mind, i am still new to the concept of using Stack for container classes and Implicit Sharing, can you throw some light into it?

[Lykurg]

First you should decide which container class is best for you. See http://doc.trolltech.com/4.5/contain...mic-complexity in detail.

Then if you use a QVector, your approach to delete is correct. With QList it is just a line shorter. nothing else.

And then you should really read about the shared concept in Qt. is the "design constraints" your decision? If so chance it now. It can save you some some pain in the ass later: http://doc.trolltech.com/4.5/shared.html#shared-classes.

[linuxdev]

Thanks for the reply

Can u suggest me, a good design to use for this case, instead of creating a Qvector in the heap. Can i create a QVector(or any container best maching my requirement) with some known size instead of dynamic heap creation?

My requirement is to have,
N different arrays capable to hold at-most 50 items, for each key.

[Lykurg]

My requirement is to have,
N different arrays capable to hold at-most 50 items, for each key.

That's not enough information to decide. If you insert an item. Where will you insert? Always at the end/front. In the middle? And when you want access an item. Where is that item? Front/back/middle? Different, or always the same location?

[linuxdev]

Sorry for not providing the information at first.


Here is complete detail:

a. The data is received from network at continuos intervals
b. The data has an unique Id/key associated with it.
c. For each key i receive, next data/update in a interval of 3secs.
d. I need to store a maximum of 50 updates for each data, and many such data and the data is always stored sequentially.
e. i just access these data only sequentially.
f. There can be atmost 200 such unique data in my application at any point of time.
g. i need to do an efficient search based on the Unique ID.

If you need more information, i am working on Radar Display Software, and data called Track data (Detections) is recevied from radar at specific intervals, say 3secs.

Each Track has an ID/Key associated with it and there can be atmost 200 Tracks and any point of time in the application and hence in my display and database.

for each Track, i need to store trial information, this is nothing but the history for a particular track. This data is just stored sequentially.

My current design is,
I have an QHash, which has TrackId as key and pointer to QVector as Value.
Each time i receive a new track, i create a new QVector and append the new track data as the first element and on every update i just append to the end.

When a data for already existing key is received, i just append to the end of QVector which already exists.

You can get more information from the function, "track+dbase" i have posted earlier.

Hope you find this useful...

[Lykurg]

Ok, QHash is the best for providing fast lookups and if you don't need the maximum 200 items to be ordered, perfect.

The point with QVector is, that it stores all items in adjacent positions in memory. So to avoid much coping use QVector::reserve(). But then you need a "lot" of space. Don't know how much is available (embedded?). If I haven't make a mistake while counting 200 keys with 50 items is only about 5 MB of disk space, so I think you can keep your QVector.
But if you store pointers to the items TTrack, then I would take a QList.

QHash<int, QList<TTrack*> > foo; //first choise
QHash<int, QVector<TTrack> > bar; //second choise

QHash<int, QList<TTrack*> > foo; //first choise
QHash<int, QVector<TTrack> > bar; //second choise

To copy to clipboard, switch view to plain text mode 

And if you need to store pointers to the vector, don't forget delete on it when removing a key from QHash...

[linuxdev]

Thanks i have taken second choice ...