Jump to content

  • Log In with Google      Sign In   
  • Create Account


Can't delete a pointer that I declared in a constructor


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
17 replies to this topic

#1 Conny14156   Members   -  Reputation: 258

Like
0Likes
Like

Posted 22 July 2013 - 06:37 AM

Hi,

I have this really wierd problem, where its not possible for me to free a certain memory.

 

My Initializilation code start like

Global->EntityController.NewEntity("test");
Entity* TestObject;
TestObject = Global->EntityController.RequestEntity("test");
TestObject.name = "blan";
TestObject->AddComponent_S(cFactory.ComponentInstantiate_S<RenderComponent>());
TestObject->AddComponent_S(cFactory.ComponentInstantiate_S<ModelHandler>());
TestObject->GetComponent_S<RenderComponent>()->StandardBox();

And heres the body of the Newentity function body

void EntityHandler::NewEntity(std::string name)
{
	Entity testing;
	testing.name = name;
	TotalEntity.push_back(testing);
}

I Just made a simple one where I just push back the value into a vector. The constructor for my Entity is

Entity::Entity()
{
	FactoryComponent cFactory;
	AddComponent(cFactory.ComponentInstantiate<TransformComponent>());
	AddComponent_S(cFactory.ComponentInstantiate_S<TransformComponent>());//<----- This one is the problem
}

I use a simple template system where I just return a "new" pointer into a vector inside Entity

template<class Component>
Component* ComponentInstantiate_S()
{
	Component* tC;	
	tC = new Component;
	return tC;
}

Works great for me, and I get no error. And I have no problem getting it added into my vector

void Entity::AddComponent_S(BaseComponent* type)
{
	if(ComponentList_S.count(type->getTypeName()) == 0)
	{
		type->setHolder(this);
		ComponentList_S.insert(std::make_pair(type->getTypeName(),type));
	}
}

The way I made it, is that all entity need to have a transform component, so I add one transform component whenever a entity gets initlized. 

The wierd part is that, I get no error and the transform component gets added to the vector. And seeing how I use push back on "testing" the object get copied into the vector. at the end of the function testing already reach the end of its life and ehm "die" and will call upon the destructor. The wierd part here is that I get no error.

for(std::map<std::string,BaseComponent*>::iterator it = ComponentList_S.begin(); it != ComponentList_S.end();it++)
{
	if(it->second)
	{
		it->second->Release();
		delete it->second;
		it->second = 0;
	}
}
ComponentList_S.clear();

But for some reason, just this one entity "Blan" where I creates with "NewEntity()" it pop up a error saying that I can't access this memory when the program reach to the delete section of the code.

I also want to point out that I added render component and modelhander outside of the constructor and both memory got freed without error's.

And when I try to remove the auto add transform component from the constructor and manual add it under the initializilation function

Global->EntityController.NewEntity("blan");
Entity* TestObject;
TestObject = Global->EntityController.RequestEntity("blan");

TestObject->AddComponent_S(cFactory.ComponentInstantiate_S<TransformComponent>());//<--- Like so

TestObject->AddComponent_S(cFactory.ComponentInstantiate_S<RenderComponent>());
TestObject->AddComponent_S(cFactory.ComponentInstantiate_S<ModelHandler>());
TestObject->GetComponent_S<RenderComponent>()->StandardBox();

Than it works as "intended" except the part that I need to add it manually.

 

Some help and info on this matter would be greatly appecirated,  and sorry for the bad english, not my mother tongue. 

 

Thanks you 


Edited by Conny14156, 22 July 2013 - 06:43 AM.


Sponsor:

#2 Ashaman73   Crossbones+   -  Reputation: 6871

Like
1Likes
Like

Posted 22 July 2013 - 06:58 AM


But for some reason, just this one entity "Blan" where I creates with "NewEntity()" it pop up a error saying that I can't access this memory when the program reach to the delete section of the code.

Your NewEntity method create the entity on the stack and not on the heap:

Entity testing;
	testing.name = name;
	TotalEntity.push_back(testing);

Once your NewEntity methods returns, the testing entity will be gone resulting most likely in a followup error once you try to access/delete it.

 

The correct way would be something like this:

Entity* testing = new Entity();
    testing->name = name;
    TotalEntity.push_back(*testing);

Update:

I know, that a vector::push_back creates a copy of the data, but this does not prevent you from using pointers as type of the vector. Can you give more information, like the definition of TotalEntity.

 

Update:

It seems that I'm on the wrong track...


Edited by Ashaman73, 22 July 2013 - 07:18 AM.


#3 Conny14156   Members   -  Reputation: 258

Like
0Likes
Like

Posted 22 July 2013 - 07:36 AM

Yeha. From the begining I started to expriment with vector containing pointers instead for object/value(whatever the word is XD),but somewhere along I just thougth mayaswell do it without pointers cause I didn't need to smile.png, and my NewEntity doesn't return anything :l, maybe I should make it return a bool sometime in the future to know if I succed or not. And yeha ask anything that may help you guys help me solving this :l 

 

P.S Edit:

 
private:
std::vector<Entity> TotalEntity;
TotalEntity is just a vector of entity

Edited by Conny14156, 22 July 2013 - 07:42 AM.


#4 Serapth   Crossbones+   -  Reputation: 5243

Like
0Likes
Like

Posted 22 July 2013 - 09:11 AM

Post the code for RequestEntity

#5 Conny14156   Members   -  Reputation: 258

Like
0Likes
Like

Posted 22 July 2013 - 06:06 PM

Entity* EntityHandler::RequestEntity(std::string name)
{
	std::vector<Entity>::iterator it;

	if(!name.empty())
		name = Renaming(name);	
	for(int i = 0;i < TotalEntity.size();i++)
		if(TotalEntity[i].name == name)
			return &TotalEntity[i];
	return NULL;

}

Its pretty simple I just for loop the whole vector and compare the two name, and return that value.



#6 Pink Horror   Members   -  Reputation: 1115

Like
0Likes
Like

Posted 22 July 2013 - 07:19 PM

You are crashing in the destructor for a BaseComponent, right? Is there a reason you choose not to share that class?



#7 Conny14156   Members   -  Reputation: 258

Like
0Likes
Like

Posted 22 July 2013 - 09:54 PM

You are crashing in the destructor for a BaseComponent, right? Is there a reason you choose not to share that class?

Am crashing at 

for(std::map<std::string,BaseComponent*>::iterator it = ComponentList_S.begin(); it != ComponentList_S.end();it++)
{
	if(it->second)
	{
		it->second->Release();
		delete it->second; //<--- Crashing here
		it->second = 0;
	}
}
ComponentList_S.clear();

And this part is not a deconstructor, it that before I end the program I clean up and free some memory


class BaseComponent
{
public:	
	BaseComponent();
	virtual ~BaseComponent(){}	
	virtual void Update(){};
	virtual void Release();
	std::string getTypeName();
	void setHolder(Entity*);
	Entity* thisObject;
	std::string typeName;
};

Basecomponent class don't have anything in it :l but here it is.



#8 Paradigm Shifter   Crossbones+   -  Reputation: 5212

Like
0Likes
Like

Posted 22 July 2013 - 09:59 PM

What does Release() do for the dodgy object?


"Most people think, great God will come from the sky, take away everything, and make everybody feel high" - Bob Marley

#9 Conny14156   Members   -  Reputation: 258

Like
0Likes
Like

Posted 22 July 2013 - 10:10 PM

What does Release() do for the dodgy object?

Well, Its usually did what the deconstructor does. But I moved the code from the release function > deconstructor for each class. It just a leftover code for now, am still quietly new to this so I "experiment" and do alot of stupid stuff. at the time I didn't think I could use the deconstructor to release my object so I did it manually :l. 

But for the transform component, It doesn't have a override release(), so it does nothing actually 



#10 Servant of the Lord   Crossbones+   -  Reputation: 17961

Like
0Likes
Like

Posted 22 July 2013 - 10:29 PM

Adding 'TransformComponent' in the Entity's constructor like you want, try this:

Global->EntityController.NewEntity("blan"); //TransformComponent already created in Entity's constructor.
Entity* TestObject = Global->EntityController.RequestEntity("blan");

assert(TestObject->GetComponent_S<TransformComponent>(), "Oops, it was already deleted");

See whether the assert is triggered or not.


It's perfectly fine to abbreviate my username to 'Servant' rather than copy+pasting it all the time.

[Fly with me on Twitter] [Google+] [My broken website]

All glory be to the Man at the right hand... On David's throne the King will reign, and the Government will rest upon His shoulders. All the earth will see the salvation of God.                                                                                                                                                            [Need web hosting? I personally like A Small Orange]
Of Stranger Flames - [indie turn-based rpg set in a para-historical French colony] | Indie RPG development journal


#11 Servant of the Lord   Crossbones+   -  Reputation: 17961

Like
0Likes
Like

Posted 22 July 2013 - 10:33 PM

Oh, and in this piece of code:

Entity::Entity()
{
	FactoryComponent cFactory;
	AddComponent(cFactory.ComponentInstantiate<TransformComponent>());
	AddComponent_S(cFactory.ComponentInstantiate_S<TransformComponent>());
}

'cFactory' is created on the stack there. It's not a member-variable of Entity, which means cFactory will be destroyed and its destructor called. Is that intentional?


It's perfectly fine to abbreviate my username to 'Servant' rather than copy+pasting it all the time.

[Fly with me on Twitter] [Google+] [My broken website]

All glory be to the Man at the right hand... On David's throne the King will reign, and the Government will rest upon His shoulders. All the earth will see the salvation of God.                                                                                                                                                            [Need web hosting? I personally like A Small Orange]
Of Stranger Flames - [indie turn-based rpg set in a para-historical French colony] | Indie RPG development journal


#12 Conny14156   Members   -  Reputation: 258

Like
0Likes
Like

Posted 22 July 2013 - 10:40 PM

I found the problem now, I think.


void Entity::AddComponent_S(BaseComponent* type)
{
	if(ComponentList_S.count(type->getTypeName()) == 0)
	{
		type->setHolder(this);//<----- This line
		ComponentList_S.insert(std::make_pair(type->getTypeName(),type));
	}
}

When I add a component, I also assign the entity that the component is added to with the line above,

The problem is when I push_back its copy the value, which mean it also copy the soon to be "dead" entity, the testing object.

So in my vector its a "new" entity, but with the same value of my old one, so inside the transform component its holds the pointer towards the "dead" entity. so when I try to delete the object it gets a error as I guess it goes crazy when it look at that pointer. Which may explain why it works when I add the transform component outside the constructor cause the "holder " of the transform component gets a correctly pointer towards a "living" holder, but should it really be a problem? 

 

I guess I will try to make the vector of pointers instead of value's, so that nothing changes, will be back soon when I gets some good results



#13 Conny14156   Members   -  Reputation: 258

Like
0Likes
Like

Posted 22 July 2013 - 10:43 PM

Oh, and in this piece of code:

Entity::Entity()
{
	FactoryComponent cFactory;
	AddComponent(cFactory.ComponentInstantiate<TransformComponent>());
	AddComponent_S(cFactory.ComponentInstantiate_S<TransformComponent>());
}

'cFactory' is created on the stack there. It's not a member-variable of Entity, which means cFactory will be destroyed and its destructor called. Is that intentional?

About that yeha its intentional, I was deciding if I wanted to have one factory inside my global class header, but I won't be really needing the same one as its only a creating method, so instead of wasting some memory I decided to just declare a new one whenever I need to add a component, still in the exprimenting phase. I don't think its a bad idea to let the factory die? as its doesn't contribute to anything after it creates the component



#14 Conny14156   Members   -  Reputation: 258

Like
0Likes
Like

Posted 22 July 2013 - 10:47 PM

Adding 'TransformComponent' in the Entity's constructor like you want, try this:

Global->EntityController.NewEntity("blan"); //TransformComponent already created in Entity's constructor.
Entity* TestObject = Global->EntityController.RequestEntity("blan");

assert(TestObject->GetComponent_S<TransformComponent>(), "Oops, it was already deleted");

See whether the assert is triggered or not.

Just tested it, No messages comes up. As I think it should if there is a problem right?



#15 Servant of the Lord   Crossbones+   -  Reputation: 17961

Like
0Likes
Like

Posted 22 July 2013 - 11:29 PM

If you do this:

BaseComponent *component = new DerivedComponent;
BaseComponent notAPointer = *component;

You are 'slicing' the object.

 

How many files is your project? If it's only two or three files, could you upload them to someplace like Ideone.com, and post the links, so we can see the entire code? I feel like I'm looking at a puzzle with half the pieces missing. smile.png


It's perfectly fine to abbreviate my username to 'Servant' rather than copy+pasting it all the time.

[Fly with me on Twitter] [Google+] [My broken website]

All glory be to the Man at the right hand... On David's throne the King will reign, and the Government will rest upon His shoulders. All the earth will see the salvation of God.                                                                                                                                                            [Need web hosting? I personally like A Small Orange]
Of Stranger Flames - [indie turn-based rpg set in a para-historical French colony] | Indie RPG development journal


#16 Ashaman73   Crossbones+   -  Reputation: 6871

Like
0Likes
Like

Posted 22 July 2013 - 11:59 PM

What do these two methods ?

        it->second->Release();
        delete it->second; //<--- Crashing here

Ensure, that they are not overlapping, e.g. 'Release' deletes internal data while the destructor does it too. Maybe you forgot to set a pointer to NULL after deleting it in Release and you try to delete it a second time in the destructor.



#17 Conny14156   Members   -  Reputation: 258

Like
0Likes
Like

Posted 23 July 2013 - 12:14 AM

If you do this:

BaseComponent *component = new DerivedComponent;
BaseComponent notAPointer = *component;

You are 'slicing' the object.

 

How many files is your project? If it's only two or three files, could you upload them to someplace like Ideone.com, and post the links, so we can see the entire code? I feel like I'm looking at a puzzle with half the pieces missing. smile.png

Yeah I know about the slicing problem, but It should be fine right? as its just basepointer that points to a derived pointer, and when I try to retrieve the component I just cast it upwards again.

template<class T_S>
T_S* GetComponent_S()	
{
	std::string test = typeid(T_S).name();
	std::map<std::string,BaseComponent*>::iterator it;
	//"Class" + " "
	for(int i = 0; i < 6 ;i++)
	{
		test.erase(test.begin());
	}
	it = ComponentList_S.find(test);
	
	if(it != ComponentList_S.end()) 
	{
		T_S* ptTComponent;
		ptTComponent = (T_S*)it->second;
			return ptTComponent;
	}
	else 
		return NULL;
	//*/
}

And I don't mind uploading my project, but I don't think you want me to :l seeing as there's about 56 files, but most of files is not related toward this,and its probably one of the uggliest thing you ever see in you lifetime :x. as this project is just a learning project for me. Where I have been doing "crazy" and stupid stuffs, to help me try understanding coding.

 

But I think I solved the problem now, Instead of having a vector<Entity> I just made it a vector<Entity*> to prevent it from copying a dead object.

 

if you still want me to upload it I can or perhaps just the files that you may want?, and am still intresterd in knowing why I get that error as it may help me prevent myself from doing the same in the future. 

 

 

P.S

if it matter I get this same reading violation adress 0xfeeefeee.

 

 

 

P.S.S

Another theory which may be the problem

 

 

When I use this code

Global->EntityController.NewEntity_S("blan");

It will Trigger 

void EntityHandler::NewEntity(std::string name)
{
	Entity testing;
	testing.name = name;
	TotalEntity.push_back(testing);
}

Here the testing entity gets push_back into the Vector, but the vector "copies" the entity. so after the end of this function, testing will die, so it will call upon the destructor. Seeing as the destructor is been call upon, it will free the memory of the transformcomponent that gets added in the constructor.

So when the program ends it will try to delete the same memory, which already been deleted. and seeing how its "technially" two diffrent object, like twin. the testing transform component gets "zero-fied"(Not sure if this is even a word, I mean that the pointer gets equal to = 0 ), the second twin doesn't as it copies the state of the other twin before it.

Which explains why when I made the vector from <Entity> to <Entity*> it suddenly works again, cause the same memory adress only gets "deleted" once.

 

P.SSS

if the text made no sense is that when I was going to upload the idea struck me and I started write this second section :x. sorry for all the confusingness.

 

oh while am at it.

 

If we say that I still want to go with a vector<Entity> instead of pointers version, what is a good idéa to prevent this? seeing that it won't work just to see if the pointer is null or not. is there a way to check if its possible to see if a certain adress already been deleted?



#18 Conny14156   Members   -  Reputation: 258

Like
0Likes
Like

Posted 23 July 2013 - 12:16 AM

What do these two methods ?

        it->second->Release();
        delete it->second; //<--- Crashing here

Ensure, that they are not overlapping, e.g. 'Release' deletes internal data while the destructor does it too. Maybe you forgot to set a pointer to NULL after deleting it in Release and you try to delete it a second time in the destructor.

Yeha, Release did what the deconstructor did, so I just copied the whole release function inside the destructor, but now it just leftover code. Doesn't matter if its there or not :x sorry for confusing you guys, but its still there because I may have some use for it sometimes in the future :l






Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS