Sign in to follow this  

Quick std::vector question (pointer related)

This topic is 4663 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hello, Just a small question of it's anwser has been eluding me for a while. In my projects I'm using the memory manager from fluid studio's (thanks guys!). Now in the log I always get memory leaks on the following 2 lines of code:
	Texture* T = new Texture();
	TextureContainer.push_back(T);

This is how I create my TextureContainer (it might not be the best way...):
std::vector<Texture*> TextureContainer;

It's located inside a class (the above 2 lines are also inside the class). Everything works fine btw. But the memory leak the program is reporting is worrying me a little bit... I know it's nothing (well, not for 3, small, textures, but what if it get's a 100 textures). So how do I prevent it? Does the compiler (or program) create a new instance of it? So I can do just this:
	Texture* T;
	TextureContainer.push_back(new Texture());
	T = TextureContainer[TextureContainer.size()-1];

It doesn't seem to be making any diffrence. Also, how much is true that this memory manager is useless when using threaded programming? My current code isn't, but I'd like to keep using this later on. Thank you for your time, mldaalder

Share this post


Link to post
Share on other sites
You need to delete the contens of your vector when you're done. When the vector's destructor is called it calls tge destructor for all of the elements in it, but it doesn't actually call delete on them (Otherwise you couldn't store non-pointers in it).

From what I've seen of mmgr, it doesn't use critical sections or locks of any kind. So if you use it in a multithreaded app, you'll get all kinds of corruption.
So no, it's not suitable for multithreaded apps, although you could easily make it so (Add an EnterCriticalSection() and LeaveCriticalSection() to the operator news and deletes).

Share this post


Link to post
Share on other sites
So the following won't clear it up?


TextureContainer.erase(TextureContainer.begin(), TextureContainer.end());
TextureContainer.clear();



And thanks for the info about EnterCriticalSection and LeaveCriticalSection functions! I haven't tried MultiThreaded programming, but I am going to, I'd love to see a working progress bar for my textures.:P

Share this post


Link to post
Share on other sites
Yup. What RenderTarget said. Calling erase() or clear() or anything like that just removes the pointers from the vector (all other STL containers work like this too). If you call new yourself, the STL will never call delete for you. You have to call delete yourself.

Someone here is bound to suggest smart pointers, since this is exactly the kind of thing they're designed for. I never use them myself, I like to make things difficult for myself [smile], so I can't really advise on any

More info about Critical Sections (slightly offtopic):
A critical section object is just a resource. You create it once (usually at application startup) with InitializeCriticalSection, and free it once (usually at application shutdown) with DeleteCriticalSection. If you have a piece of code that you only want one thread to be able to access at once (E.g. when you're adding to a linked list, like mmgr does when you call it's overloaded new), you call EnterCriticalSection. If a thread is in the section and another thread tries to get in (it gets to the EnterCriticalSection() call), it'll block until the first thread comes out of the block.
Once you've finished whatever code needs protected, you call LeaveCriticalSection. Make sure that if you have a function that calls EnterCriticalSection(), it calls LeaveCriticalSection() before exiting. A handy way to do this is to make a mutex class thet calls EnterCriticalSection() in the constructor, and LeaveCriticalSection() in the destructor. Then, when the object goes out of scope (e.g. when the function ends), the object gets destroyed, and your critical section is left.

Share this post


Link to post
Share on other sites
Thanks for the info guys!

I'm currently trying a couple of ways I think it could be done (so far no luck).
But I'll get there (eventually).

Just curiously, how would you implement it?
Using std::vector or something else (anything short of your own implementation, that's something I'm thinking of doing later on when I get to octrees).

Share this post


Link to post
Share on other sites
You need to create an iterator, and litterally delete every single thing you new.

std::vector<Texture *>::iterator it = TextureContainer.begin();
for(it; it != TextureContainer.end(); ++it)
{
delete *it;
}
TextureContainer.clear();


First, notice how I iterate using the vector::iterator subclass. Next, notice that I have to dereference the iterator to delete it (it would return a Texture *, which is what I want to delete.)

Good luck!

Share this post


Link to post
Share on other sites
lol

That's what I just (a minute ago) did.:P

Though it seems to wreck mmgr (it hit this: "If you hit this assert, you tried to deallocate RAM that wasn't allocated by this memory manager.").

Well, wreck is a to strong a word...

Wait, what if I press Continue, instead of Abort (does that).

Yah! Now it doesn't make the entry.

Thanks!
Maybe I'll just modify the mmgr code, so that it just says something like this:
"Deleted *Some Identifier* memory not allocated by this memory manager at line___"

Share this post


Link to post
Share on other sites
Quote:
Original post by visage
You need to create an iterator, and litterally delete every single thing you new.

std::vector<Texture *>::iterator it = TextureContainer.begin();
for(it; it != TextureContainer.end(); ++it)
{
delete *it;
}
TextureContainer.clear();


That's a nice example, but I much prefer functors for this kind of thing.

struct delete_pointer
{
template < typename ValueT > void operator()( ValueT * pointer ) const
{
delete pointer;
}
};

std::for_each( TextureContainer.begin() , TextureContainer.end() , delete_pointer() );


Pros:
1) Useage does not have to specify iterator types. This means that if TextureContainer is changed from a std::vector< Type1 * > to a std::vector< Type2 * > or std::list< ... >, the code will not break.
2) Less typing. Sure, declaring the whole delete_pointer struct is a bit wordy, but you only have to write it once. If you have it in your library, you can easily delete LOTS of data:

std::for_each( container1.begin() , container1.end() , delete_pointer() );
std::for_each( container2.begin() , container2.end() , delete_pointer() );
std::for_each( container3.begin() , container3.end() , delete_pointer() );


3) Functors are damn sexy.

Con:
Dosn't work correctly if the pointers are a mix of things allocated with new, and other things allocated with new[].
Dosn't work with new[]ed stuff period. This is why you have:

struct delete_array
{
template < typename ValueT > void operator()( ValueT * pointer ) const
{
delete[] pointer;
}
};

Share this post


Link to post
Share on other sites
I give you my auto-deleting vector container :)

It's simple, dumb and easy to use, it's just one header file and removed a lot of headaches from storing pointers that need to be deleted later.


#ifndef _templateVectorOfPtrs_HPP_
#define _templateVectorOfPtrs_HPP_

#include <vector>

/**
std::vector wrapper that will delete all of its contents when done

Usage:
Before:
std::vector<MyObject *> myVector;
myVector.push_back(new MyObject("foo"));
...
//later need to delete the content

Now:
AVectorOfPtrs<MyObject> myVector;
myVector._vector.push_back(new MyObject"foo"));
**/


template<class T>
class AVectorOfPtrs
{
public:
typedef std::vector<T*> TYPEDEF;
TYPEDEF _vector;

/**
Will call delete on all members
**/

~AVectorOfPtrs()
{
TYPEDEF::iterator it = _vector.begin();
while (it != _vector.end())
{
delete *it;
++it;
}
}
};

#endif // _templateVectorOfPtrs_HPP_





++Alex

Share this post


Link to post
Share on other sites
Thank you all for the tips and code guys!

It's very appreciated.
I've been upgrading my code all over the place.
Odly enough, I seem to be running into some problems externing the struct, oh well, I always needed to create an include with the common stuff properly set up for everything.

Though, I'm still trying to prevent it from hitting the assert of mmgr.

But that's just because it's not newed by it, but inside std itself, right?


[EDIT] I should look better next time...
There is this handy output screen in VS.:P
The asserts only happen on the deleting of char arrays, of which it's contents are just created inside the code (so no outside loading from files).
Anyone knows how to do it properly?

Share this post


Link to post
Share on other sites
Quote:
Original post by mldaalder
[EDIT] I should look better next time...
There is this handy output screen in VS.:P
The asserts only happen on the deleting of char arrays, of which it's contents are just created inside the code (so no outside loading from files).
Anyone knows how to do it properly?


If you're creating your char arrays like so:

pointer = new char[...];

You should delete them like so:

delete[] pointer;

You may want to use std::string if these char arrays are strings, as std::string will automatically worry about memory allocation and deallocation for you :).

(Example:
#include <string>
#include <iostream>
using namespace std;

int main ( int argc , char ** argv )
{
string message = "Mike";
message += " is sexy.";
cout << message << endl; //outputs "Mike is sexy."
message = "30";
cout << message << endl; //outputs "30"
} //no memory leak at the end of the program!

Share this post


Link to post
Share on other sites
The problem (was) that I was feeding char arrays like this:

Texture* myTexture = cTexManager.AddTexture("Data/MyTexture.tga");
Then I used the reference of the passed in char array for my variable, and then I deleted that, but wasn't newed by mmgr. So it hit the assert.

Now I copy that char array instead of referencing it, and it isn't hitting the assert now.

So, is that creating a new memory leak?

Share this post


Link to post
Share on other sites

This topic is 4663 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this