Sign in to follow this  
Antrim

Problem with a Resource Manager

Recommended Posts

I'm trying to utilize a variation of Scripts and Resource Management from the "FPS in DirectX" book. There is one thing in particular in the book that I wanted to change that is not working for me, and I know *what* is going on, but I'm not sure why. Hopefully someone here can throw the info my way ;) Basically, in my global engine there is a function to get a pointer to my Script Resource Manager. The function is public, and the Manager* is protected. Now I have another global class that gets the pointer to my Manager via the GetScriptManager() call, and then asks it to add a Script... ResourceManager* manager = GetScriptManager(); Script* setup = manager->Add("setup.txt"); When the Manager adds a Script, it uses the "push_back" of it's vector to store the new Script, and then returns a pointer to it. My problem is that when the pointer comes back, it is invalid. I debugged enough to see that right before the return, the most recently added element of the vector is ok, so the only thing I can think of is that there is a problem since the Manager itself (and therefore it's members) is protected. If that is the case, then I'm just not really clear on why it's member function can return a pointer, but that pointer becomes invalid. The basic jist of my code is as such (inside the Managers "Add" function): Script *resource = new Script("setup.txt"); m_Variables->push_back(*resource); return &(m_Variables->back()); I'd appreciate any explanation anyone can give as to why this happens...hopefully my description is clear enough. Thanks for any input, Antrim

Share this post


Link to post
Share on other sites
I think your problem lies here:

return &(m_Variables->back());

as m_Variables->back() will be generated as a local value, and loses it's value, when you leave the function.

Why don't you just return the iterator for your vector? That works similiar to pointers, otherwise the best approach would be to store pointers in your vector.

Share this post


Link to post
Share on other sites
Quote:

I think your problem lies here:

return &(m_Variables->back());

as m_Variables->back() will be generated as a local value, and loses it's value, when you leave the function.


back() returns a reference, which has the same address as the object inside the vector.

Pointers to objects stored in a vector are not stable: they can become invalid when you insert, add or remove items. Make sure you don't do anything at all with the vector until you are done using the pointer to the script.

The pointer to the script is stable if:
- You use a list
- You store pointers to scripts instead of scripts

Share this post


Link to post
Share on other sites
by the way:
Script *resource = new Script("setup.txt");
m_Variables->push_back(*resource);
return &(m_Variables->back());

You know you have a memory leak there?

correct it would be:

Script *resource = new Script("setup.txt");
m_Variables->push_back(*resource);

delete resource;

return &(m_Variables->back());

Share this post


Link to post
Share on other sites
Ok, I think that switching the vector to store pointers to Scripts might be the easiest fix (less code to change). Also, I hadn't thought about the implications of manipulating the vector again before I was done using the pointer I just grabbed...not sure how likely it is, but better to be safe.

I also didn't realize that sending &(m_Variables->back()) as the return value would loose its value. I thought returning it would just give me a pointer to the location of the object that was referenced by back().

I suppose where I'm confused is that a function, such as:


char* test(){
char* value = "abc";
return value;
}



would successfully return the value, even though value is a locally declared variable. However, returning &(m_Variables.back()) looses its value. Something about that just doesn't seem right to me. Maybe someone can beat some sense into me as to why this is.

As for the memory leak...heh, yeah, thanks for pointing that out. I, for some reason, was just thinking that deleting resource would get rid of the object in the vector. Forgot that it makes its own copy.

Thanks for the info,

Antrim

Share this post


Link to post
Share on other sites
char* test(){
char* value = "abc";
return value;
}


this works also only, if you are lucky...
as there is no garantuee, when the memory will be reused, where value is pointing to....

a way to get this to work is, would be:

static char* value;

or by returning a string, which will be then copied.

Share this post


Link to post
Share on other sites
Really? man, I always thought that was an ok thing to do.

Anyway, thanks for clearing stuff up for me. I just switched the vector to holding pointers and everything is working fine now. Glad I got straightened out before I completly butchered my code.

By the way, just to make sure I understand how the vector works...Now that I'm using pointers, not deleting "resource" doesn't cause a memory leak correct...doing so would actually delete the Script I'm wanting to use right?

Script* resource = new Script("setup.txt");
m_Variables->push_back(resource);
return m_Variables.back();

Just wanted to make sure I'm understanding vector correctly...in that it just makes it's own copy of whatever you push back...in this case, the pointer, which will take care of itself on function exit right?

Thanks again for all the help, it's much appreciated,

Antrim

Share this post


Link to post
Share on other sites
In the function itself you have now longer a memory leak, but
formally you have still a memory leak for your whole program ;-)

But this not really bad, as all memory still allocated by a programm will be "freed" at exit.

Correct would be somewhere a loop about your vector, where you call a delete for each element.
and then a clear() for the vector itself.

Share this post


Link to post
Share on other sites
ah, yeah, I actually do have a function in the ResourceManager that iterates through the vector and deletes each pointer, then does a clear().

I also remembered to put the "delete" call in the ResourceManager::Remove() as well, so I think all the leaks should be covered ;)

Share this post


Link to post
Share on other sites

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