Jump to content

  • Log In with Google      Sign In   
  • Create Account

Destructor in vector called too often


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
27 replies to this topic

#1 Juliean   GDNet+   -  Reputation: 2619

Like
0Likes
Like

Posted 19 August 2014 - 02:29 AM

Hello,

 

I recently experienced a very strange crash, which after inspecting further turned out to be caused by the destructor of a class being called one time too often in a certain vector creation. I havn't been able to fully determine the excact reason this is happening, so I hope some of you guys has a better idea of what could be going on. I'm using VS2013. This is the code that crashes:

const ecs::ComponentDeclaration& Sprite::GenerateDeclaration(void)
{
	const ecs::ComponentDeclaration::AttributeVector vAttributes = 
	{
		{ L"Texture", core::VariableType::STRING, offsetof(Sprite, stTexture), onSetTexture },
		{ L"Blending", core::VariableType::INT, offsetof(Sprite, blend) },
	}; // this line here

	static const ecs::ComponentDeclaration declaration(L"Sprite", vAttributes);

	return declaration;
}

The crash happens in the commented line, so appearently in the vector construction. Let me show you all related code:

struct ACCLIMATE_API ComponentDeclaration
{
public:
	typedef std::vector<AttributeDeclaration> AttributeVector;
		
//...
}

This is the declaration of the vector.

struct ACCLIMATE_API AttributeDeclaration
{
	typedef void(*SetCallback)(BaseComponent& component);

	AttributeDeclaration(const std::wstring& stName, core::VariableType type, size_t offset, SetCallback pCallback = nullptr);

	std::wstring stName;
	size_t offset;
	SetCallback pCallback;
	ICustomComponentAttributes* pCustom;

private:
	core::Variable m_default;
};

AttributeDeclaration::AttributeDeclaration(const std::wstring& stName, core::VariableType type, size_t offset, SetCallback pCallback) :
    stName(stName), pCallback(pCallback), m_default(type, false), pCustom(nullptr)
{
}

Thats the class that is being stored, with all related variables/methods. It stores a core::Variable class, which acts like a semi-variant data-container, which is defined as follows:

class ACCLIMATE_API Variable
{
public:
	static const unsigned int NO_OBJECT = -1;

	Variable(VariableType type, bool isArray);
	Variable(const Variable& variable);
	Variable(Variable&& variable);
	~Variable(void);

	Variable& operator=(const Variable& variable);
	Variable& operator=(Variable&& variable);

private:

	void* m_pData;
	VariableType m_type;
	bool m_isArray;
	unsigned int m_objectType;
};

Variable::Variable(VariableType type, bool isArray) :
	m_type(type), m_isArray(isArray), m_objectType(NO_OBJECT),
	m_pData(nullptr)
{
	ACL_ASSERT(m_type != VariableType::OBJECT);

	if(type != VariableType::UNKNOWN)
		m_pData = CallByTypeNoObject<newData, void*>();
}

Variable::Variable(const Variable& variable) :
	m_type(variable.m_type), m_isArray(variable.m_isArray), m_objectType(variable.m_objectType)
{
	m_pData = CopyData(variable.m_pData);
}

Variable::Variable(Variable&& variable) :
	m_type(variable.m_type), m_isArray(variable.m_isArray), m_objectType(variable.m_objectType)
{
	m_pData = variable.m_pData;

	variable.m_pData = nullptr;
	variable.m_type = VariableType::UNKNOWN;
	variable.m_objectType = NO_OBJECT;
}

Variable::~Variable(void)
{
	// just for simplicities sake - the actual deletion of the correct type is normally handled
	delete m_pData;
}

Variable& Variable::operator=(const Variable& variable)
{
	if(this != &variable)
	{
		m_type = variable.m_type;
		m_isArray = variable.m_isArray;
		m_objectType = variable.m_objectType;

		DeleteData();

		m_pData = CopyData(variable.m_pData);
	}
	
	return *this;
}

Variable& Variable::operator=(Variable&& variable)
{
	if(this != &variable)
	{
		m_type = variable.m_type;
		m_isArray = variable.m_isArray;
		m_objectType = variable.m_objectType;

		m_pData = variable.m_pData;
		variable.m_pData = nullptr;
		variable.m_type = VariableType::UNKNOWN;
		variable.m_objectType = NO_OBJECT;
	}

	return *this;
}

Now the crash happens inside this classes ctor. Obviously, m_pData is destroyed more then once. The call hierachy looks as follows:

 

- ctor of first line is being called (line in the first code snipped)

- ctor for second line is being called

- copy-ctor is called from the object of the first line

- copy-ctor is called from the object of the second line

- Destructor is called on the first object

- Destructor is called on the second object

- Destructor is called on the second object again - crash!

 

Now the reason I know that the destructor is called again, and not m_pData is somehow deleted twice is that by setting m_pData to nullptr in the destructor, the crash resolves. Also note that it is probably not an issue with the Variable-class, as the void* m_pData has already been part of the AttributeDeclaration-class before, as well as part of some other class.

 

_____________________________________________________

 

So, thats about all the information I can give. I can't make any sense of the vector-code, so debugging it further is rather out of the question. My question is now: Does someone see something obviously wrong, or knows/has some idea what is going on? Appearently I *could* fix it by simply putting in the nullptr-set in the ctor, but thats way to hackish and obviously just hides the problem. One thing I find interesting is that the move-ctor isn't called, as it is almost always when dealing with this kind of vector-emplacement, maybe thats related to the problem?

 

 



Sponsor:

#2 Ashaman73   Crossbones+   -  Reputation: 7513

Like
2Likes
Like

Posted 19 August 2014 - 03:27 AM

Hmmm.... following idea (might be wrong tongue.png ):

 

This here

	const ecs::ComponentDeclaration::AttributeVector vAttributes = 
	{
		{ L"Texture", core::VariableType::STRING, offsetof(Sprite, stTexture), onSetTexture },
		{ L"Blending", core::VariableType::INT, offsetof(Sprite, blend) },
	}; // this line here

will most likely create two attributes first, then use the assignment operator to copy  the reference to vector. After this line you have 4 reference of two attributes, two on the "local stack" and two in the vector . Therefor 2 references will share the same m_pData, which will be deleted when leaving the method (two local refernce on stack and the const, but not static vAttributes living on the stack too). 



#3 Juliean   GDNet+   -  Reputation: 2619

Like
0Likes
Like

Posted 19 August 2014 - 03:33 AM


will most likely create two attributes first, then use the assignment operator to copy the reference to vector. After this line you have 4 reference of two attributes, two on the "local stack" and two in the vector . Therefor 2 references will share the same m_pData, which will be deleted when leaving the method (two local refernce on stack and the const, but not static vAttributes living on the stack too).

 

The thing is, the copy-ctor already duplicates m_pData. I missed to include the "CopyData"-method, but thats what it does. So just by the execution, every attribute would have its own "m_pData". Also, like I said, if I do this:

Variable::~Variable(void)
{
     delete m_pData;
     m_pData = nullptr; // that line here fixes it
}

Then the crash goes away. So it appears to be the same instance being deleted twice, or at least the same "shared memory". Also, it crashes way before leaving the method, in the closing bracket line for the vector initializer list. So unless thats what you meant I think you are unfortunately wrong. Still, thanks for the input, I really feel totally screwed up on this one :/

 

any other ideas?


Edited by Juliean, 19 August 2014 - 03:49 AM.


#4 Ashaman73   Crossbones+   -  Reputation: 7513

Like
2Likes
Like

Posted 19 August 2014 - 04:43 AM

Best to debug it, try to set breakpoints in the destructor and eventually a breakpoint on the datablock (Variable object) which seems to get deleted a second time. Take a careful look on the call stack to check if really this object got deleted a second time.


Edited by Ashaman73, 19 August 2014 - 04:46 AM.


#5 Juliean   GDNet+   -  Reputation: 2619

Like
0Likes
Like

Posted 19 August 2014 - 05:55 AM


Best to debug it, try to set breakpoints in the destructor and eventually a breakpoint on the datablock (Variable object) which seems to get deleted a second time. Take a careful look on the call stack to check if really this object got deleted a second time.

 

I'll give it another shot. I think the problem was that the whole internal call-stack was shown greyed out as "external" in the call-stack, probably due to the initializer list. I'll see if I can get the full call-stack/symbols. In the meantime, if anyone notices anything wrong in the original code, I'd be very happy (won't get back home until in a few hours anyways). Thanks.



#6 SeanMiddleditch   Members   -  Reputation: 5872

Like
7Likes
Like

Posted 19 August 2014 - 11:50 AM

Your assignment operators look broken to me.

In the copy assignment operator, you copy some metadata values and then call DeleteData. I don't know what that function does or how/if it uses the metadata, but it seems suspect.

In your move assignment operator, you never call DeleteData, so you're leaking the old data whenever you assign an rvalue-ref to it.

This a good example of where the "rule of zero" is helpful. The "rule of five" says you must implement all of the copy constructor, move constructor, both assignment operators, and the destructor, if you implement any of them. The rule of zero says you shouldn't implement any of them because the compiler-generated defaults should just work. If you replace your void* pData with something like std::unique_ptr using a custom deleter, things should Just Work™.

#7 Juliean   GDNet+   -  Reputation: 2619

Like
0Likes
Like

Posted 19 August 2014 - 12:33 PM

Ok, so I looked into it, it definately is the dtor of the same object being called twice. I don't get to see the full source/assemble, but the this-pointer in both dtor calls is identically.

So I don't know whats up with that and why/how it doesn't also crash in the "std::wstring" that the class stores, I quess that does the same thing that I currenclty do with clearing the pointer to nullptr at the end of the dtor...

 

 

 


Your assignment operators look broken to me.

In the copy assignment operator, you copy some metadata values and then call DeleteData. I don't know what that function does or how/if it uses the metadata, but it seems suspect.

In your move assignment operator, you never call DeleteData, so you're leaking the old data whenever you assign an rvalue-ref to it.

 

DeleteData() does indeed call the correct delete function based on the "m_type" value, so the assignment of the meta-data is vital for it to work. Good catch that I forgot to call it the second time around. Though none of those are actually called, I added them in vein to see if maybe there was something wrong going on here.

 

 

 


This a good example of where the "rule of zero" is helpful. The "rule of five" says you must implement all of the copy constructor, move constructor, both assignment operators, and the destructor, if you implement any of them. The rule of zero says you shouldn't implement any of them because the compiler-generated defaults should just work. If you replace your void* pData with something like std::unique_ptr using a custom deleter, things should Just Work™.

 

Hm, I might give it a try. I don't know if std::unique_ptr is going to work though, since creating/copying/deleting of the data is coupled to the type-id. I also can't really see how this would solve he issue (unless by "luck" if the unique_ptr also does that nullptr-assignment at the end). I'll give it a try, but I'd really like to know what causes this in the first place... there is nothing wrong in my ctor/copy-ctor, right?


Edited by Juliean, 19 August 2014 - 12:39 PM.


#8 Aardvajk   Crossbones+   -  Reputation: 5982

Like
3Likes
Like

Posted 19 August 2014 - 01:39 PM

When the debugger fails, there is nothing better than a shedload of std::cout << _LINE_. Think you really need a solid grasp on exactly what the program flow is doing here.

Sean quite right of course. A smart pointer can do all this for you.

#9 Juliean   GDNet+   -  Reputation: 2619

Like
0Likes
Like

Posted 19 August 2014 - 02:58 PM


When the debugger fails, there is nothing better than a shedload of std::cout << _LINE_.

 

Unfortunately, that won't do much here eigther :/ See, I can put breakpoints in whichever code I've written that is called here, like the ctors/dtors and the actual calling code, but the debugger won't let me see what the initialization list is doing - so unless I get to go into that specific code somehow, there is no way to debug it, "classically" or by doing debug output :/

 


Think you really need a solid grasp on exactly what the program flow is doing here.

 

True, if I had that, I probably wouldn't need to ask here... but as said above, there appears to be no way to get it, since I get the actual dtor-calls, but not from where exactly the call came from... can't you generally look at source of the initalizer-list of some kind?

 


Sean quite right of course. A smart pointer can do all this for you.

 

Well it would probably hide the problem by some smart self-cleanup, but really, this would probably just hide it.  There would still be an additional dtor-call to the AttributeDeclaration-class, no matter what... and I don't like that very much. I quess for being workable I can use a smart-pointer, but I still have a bad feeling about the whole thing...



#10 SeanMiddleditch   Members   -  Reputation: 5872

Like
2Likes
Like

Posted 19 August 2014 - 03:57 PM

DeleteData() does indeed call the correct delete function based on the "m_type" value, so the assignment of the meta-data is vital for it to work. Good catch that I forgot to call it the second time around.


Which is why it is broken. You're deleting your old data using the new metadata. You want to delete the old data using the old metadata. You want to assign the metadata _after_ you delete the old value.

#11 Juliean   GDNet+   -  Reputation: 2619

Like
0Likes
Like

Posted 19 August 2014 - 04:05 PM


Which is why it is broken. You're deleting your old data using the new metadata. You want to delete the old data using the old metadata. You want to assign the metadata _after_ you delete the old value.

 

I don't even know what I was thinking. Thanks! (still no solution for the original problem though, in case someone else reads this)


Edited by Juliean, 19 August 2014 - 04:07 PM.


#12 shacktar   Members   -  Reputation: 750

Like
0Likes
Like

Posted 19 August 2014 - 04:18 PM

 


Your assignment operators look broken to me.

In the copy assignment operator, you copy some metadata values and then call DeleteData. I don't know what that function does or how/if it uses the metadata, but it seems suspect.

...

 

DeleteData() does indeed call the correct delete function based on the "m_type" value, so the assignment of the meta-data is vital for it to work. ...

 

So then shouldn't you do what SeanMiddleditch implied, and call DeleteData before you assign the meta-data in the copy assignment operator?

 

Edit: I was looking at the code (that wasn't changed). I didn't see the two posts above.


Edited by shacktar, 19 August 2014 - 05:39 PM.


#13 SeanMiddleditch   Members   -  Reputation: 5872

Like
0Likes
Like

Posted 19 August 2014 - 04:43 PM

Just to clarify: you've set breakpoints verified that both `this` and `m_pData` are the same for this duplicate call? If the `this` is the same, you've then verified that there are zero intervening calls to any of the three constructors or either assignment operator? Destructors _will not_ be called more than once on an object unless you've done something bad with the delete operator or manually invoking the destructor. My suspicion at this point would be whether your copy constructor and CopyData are actually copying anything or not. Good debugging skills would be invaluable here.

#14 Juliean   GDNet+   -  Reputation: 2619

Like
0Likes
Like

Posted 19 August 2014 - 04:57 PM


So then shouldn't you do what SeanMiddleditch implied, and call DeleteData before you assign the meta-data in the copy assignment operator?

 

Yep, thats what I'm doing now, took a time to realize though ;)

 


Just to clarify: you've set breakpoints verified that both `this` and `m_pData` are the same for this duplicate call? If the `this` is the same, you've then verified that there are zero intervening calls to any of the three constructors or either assignment operator? Destructors _will not_ be called more than once on an object unless you've done something bad with the delete operator or manually invoking the destructor. My suspicion at this point would be whether your copy constructor and CopyData are actually copying anything or not. Good debugging skills would be invaluable here.

 

Yes to all. Thats actually how it appears to be happening - two ctors are being called, followed by two copy-ctors, followed by 3 dtors, 2 on the same object. I noticted the same thing happens in other code-pieces with unrelated classes of the same scheme, using the same type of intializer list. The copy is working as expected, because the classes do work with those same copy-ctors in other unrelated parts of the code, which copies way more than just here. Also notice the "fix":

AttributeDeclaration::~AttributeDeclaration(void)
{
     delete m_pDefault; // changed to pointer for testing purpose in the main class
     m_pDefault = nullptr; // THIS fixes it!
}

"AttributeDeclaration" is the class being stored in the vector. So if there really was a second pointer to "m_pDefault" in some other instance of the same class, this would not fix it Since both dtor-calls go to the same object/memory-area, setting m_pDefault to nullptr after the first dtor-call makes the next one pass. As weird as it appears, the dtor simply is called on that instance 2 times. For whatever reason there might be... -.-

 


Good debugging skills would be invaluable here.

 

I usually don't have a problem with debugging, unless the stupid debugger won't show me the callstack. I can say that given the circumstances I have done anything possible, I really hoped when posting that someone just was like "yeah, I had that one once too, its that and that", but discussing possiblities also helps!



#15 SeanMiddleditch   Members   -  Reputation: 5872

Like
2Likes
Like

Posted 19 August 2014 - 06:55 PM

So if there really was a second pointer to "m_pDefault" in some other instance of the same class, this would not fix it


If you do bad things with pointers, it's not at all impossible for things like a destructor to be invoked at an improper time, or for some broken copy/move constructor to fail to reinitialize m_pData properly.

but discussing possiblities also helps!


So, possibilities... what compiler? Have you tried others? Initializer lists are new enough there may just be a bug in your compiler, though it's exceeding unlikely given how simple of a use case you have here.

If you say you've seen this happen elsewhere, I would _love_ to see a minimal test case posted up on StackExchange or your compiler's bug tracking site, see if someone smarter than us can sort it out. smile.png

#16 L. Spiro   Crossbones+   -  Reputation: 13600

Like
3Likes
Like

Posted 19 August 2014 - 09:51 PM

const ecs::ComponentDeclaration& Sprite::GenerateDeclaration(void)
{
	const ecs::ComponentDeclaration::AttributeVector vAttributes = 
	{
		{ L"Texture", core::VariableType::STRING, offsetof(Sprite, stTexture), onSetTexture },
		{ L"Blending", core::VariableType::INT, offsetof(Sprite, blend) },
	}; // this line here

	static const ecs::ComponentDeclaration declaration(L"Sprite", vAttributes);

	return declaration;
}

One of the most important things you did not provide is what the constructor for ecs::ComponentDeclaration is doing here. Is it keeping a reference? Making a copy?
Based on your description of the constructors called I assume it is making a copy.


In any case the code here doesn’t make sense. declaration only gets initialized once while vAttributes will be created and destroyed on every call.

vAttributes should be static as well for this to make any sense.

Although this will make your problem go away, it only hides the issue, so do it only after you have solved the problem.

 

 

If you want to debug this and your debugger is being a pain, why don’t you use ::printf()?

For each form of copy, ::printf() the copy method/operator being called and the this pointer, along with anything else that might be useful (such as the source of the copy, etc.)

 

 

By the way, you never set a value for m_isArray on the moved object in your move copies.  Just to be safe.

 

 

L. Spiro


Edited by L. Spiro, 19 August 2014 - 09:55 PM.

It is amazing how often people try to be unique, and yet they are always trying to make others be like them. - L. Spiro 2011
I spent most of my life learning the courage it takes to go out and get what I want. Now that I have it, I am not sure exactly what it is that I want. - L. Spiro 2013
I went to my local Subway once to find some guy yelling at the staff. When someone finally came to take my order and asked, “May I help you?”, I replied, “Yeah, I’ll have one asshole to go.”
L. Spiro Engine: http://lspiroengine.com
L. Spiro Engine Forums: http://lspiroengine.com/forums

#17 Trienco   Crossbones+   -  Reputation: 2173

Like
3Likes
Like

Posted 19 August 2014 - 10:12 PM

There was a known bug when using the initializer list for containers (especially visible when storing smart pointers) which always screwed up the first element. If you didn't already, make sure you have the latest updates installed (I'm relatively certain that the problem was fixed in one of them).

 

Not necessarily your problem, but I spent all afternoon debugging why it's always the first element in my map that was invalid immediately after it's definition. Turned out plenty of other people saw similar bugs with other container types.


Edited by Trienco, 19 August 2014 - 10:13 PM.

f@dzhttp://festini.device-zero.de

#18 Juliean   GDNet+   -  Reputation: 2619

Like
0Likes
Like

Posted 20 August 2014 - 02:19 AM


If you do bad things with pointers, it's not at all impossible for things like a destructor to be invoked at an improper time, or for some broken copy/move constructor to fail to reinitialize m_pData properly.

 

For debugging purposes, I removed all that relates to this pointer here - same result : Destructor is called 2 times on first object. Having the pointer appearently only showed the error in the first place - since it isn't deleting the object twice, but really just somehow calls the destructor twice on the same memory area. Any change I make to the object really is carried over to the second call - like

AttributeDeclaration::~AttributeDeclaration(void)
{
     m_type = VaribleType::BOOL; // first m_type is STRING, in the second dtor call, m_type equals BOOL
}


So, possibilities... what compiler? Have you tried others? Initializer lists are new enough there may just be a bug in your compiler, though it's exceeding unlikely given how simple of a use case you have here.

If you say you've seen this happen elsewhere, I would _love_ to see a minimal test case posted up on StackExchange or your compiler's bug tracking site, see if someone smarter than us can sort it out. smile.png

 

Visual Studio 2013, default compiler. I might actually try the November 2013 CTP since I got it installed, but I have to fix a few things that crashes with that.

 

Actually I had the same thought,  it was a little late when I posted that last reply, but once I get home from work today, I will try to create a reproducible example that I can upload.

 


One of the most important things you did not provide is what the constructor for ecs::ComponentDeclaration is doing here. Is it keeping a reference? Making a copy?
Based on your description of the constructors called I assume it is making a copy.

 

For this issue, the ecs::ComponentDeclaration-constructor is irrelevant, since its never even called before the double-deletion crash.

 

But aside from this, yes, its making a copy. Thats also why I didn't want to make the vector static - its just a substitute for making a temporary vector inside the ctor-call. This method is also usually only called once, the returned declaration is then stored in the templated parent class in a static field - having it static here is just there so I don't have to "new" and manually destroy it afterswards.

 


If you want to debug this and your debugger is being a pain, why don’t you use ::printf()?

For each form of copy, ::printf() the copy method/operator being called and the this pointer, along with anything else that might be useful (such as the source of the copy, etc.)

 

Because it also wouldn't help really. See, I do have every information about my own classes, objects method calls, ctors, etc... while debugging here. I can set breakpoints as far as I want. The only thing I don't see is the code that is executed before calling e.g. the destructor. In the callstack, I see

 

- "AttributeDeclaration::~AttributeDeclaration(void)"

- "External" (greyed out)

- "const ecs::ComponentDeclaration& Sprite::GenerateDeclaration(void)"

- ...

 

Which means that as far as I'm aware, I can't use printf sensefully here. Stepping in/out in the debugger will also just send me around in my own code.

 


By the way, you never set a value for m_isArray on the moved object in your move copies. Just to be safe.

 

Thats actually a good point, I simply didn't know how to set it - there isn't really a "default value" here, since a variable can eigther be an array or not. Well, come to think about it, maybe due to the "false" is the valid option, since an empty variable most definately isn't an array. Quess I can apply that then, thanks.

 


There was a known bug when using the initializer list for containers (especially visible when storing smart pointers) which always screwed up the first element. If you didn't already, make sure you have the latest updates installed (I'm relatively certain that the problem was fixed in one of them).



Not necessarily your problem, but I spent all afternoon debugging why it's always the first element in my map that was invalid immediately after it's definition. Turned out plenty of other people saw similar bugs with other container types.

 

Well, thats a least a starting point. In all honesty, I was expecting some sort of issue with the initializer list, though it might (have?) just as well been a fault in my code. I try to apply all updates, then see if it still persists. Then I try to create said minimum example for recreating this problem, then we'll see - maybe its really a initializer list problem, or maybe while trying to recreate it in an external app I stumble across some problem of my code that's been overseen so far.



#19 Juliean   GDNet+   -  Reputation: 2619

Like
0Likes
Like

Posted 20 August 2014 - 11:28 AM

Ok, I found the actual cause of the bug. I'm kind of glad to tell you that the assumption about me doing bad stuff with memory was wrong - but let me first show you the minimal example that is needed to reproduce the "bug":

#include <iostream>
#include <vector>

struct AttributeDeclaration
{
	AttributeDeclaration(const std::wstring& stName) 
	{
		std::cout << "AttributeDeclartion() called.\n";
	}

	AttributeDeclaration(const AttributeDeclaration& declaration)
	{
		std::cout << "AttributeDeclartion(const AttributeDeclaration& decl) called with this: " << (int)this << " and decl: " << (int)&declaration << ".\n";
	}

	~AttributeDeclaration(void)
	{
		std::cout << "~AttributeDeclartion() called with this: " << (int)this << ".\n";
	}
};

int main(void)
{
	const std::vector<AttributeDeclaration> vFoo =
	{
		{ L"entity" },
		{ L"locked" },
	};

	std::cout << "Initializing vector done.\n";

	return 1;
}

Can you quess what actually causes the issue here? Yeah, its the string-argument in the ctor. The what...? Really, no matter how much I simplified the code, the only thing that causes this is if I put that "const std::wstring&" as first argument. If I remove it, the bug is gone.

 

I said I found the cause of the bug... but obviously not the reason. The string gets correctly constructed (as far as I see) in this code:

basic_string(const _Elem *_Ptr)
: _Mybase()
{	// construct from [_Ptr, <null>)
	_Tidy();
	assign(_Ptr);
}

and the strings value looks OK... it must have been, I've been working with those stored strings quite a while without a problem.

 

So in my example there of course will not be a crash, but you should be able to see the double deletion. So now at least the soure of the problem is determined and you have a very simple code you can run... can anyone make sense out of this behaviour? Is there anything wrong about the way I handle strings here (changed to std::string and removing the L doesn't help), or is this really some compiler issue (again, I'm using VS2013 default compiler)?



#20 SmkViper   Members   -  Reputation: 793

Like
0Likes
Like

Posted 20 August 2014 - 02:27 PM

Ok, I found the actual cause of the bug. I'm kind of glad to tell you that the assumption about me doing bad stuff with memory was wrong - but let me first show you the minimal example that is needed to reproduce the "bug":




#include <iostream>
#include <vector>

struct AttributeDeclaration
{
	AttributeDeclaration(const std::wstring& stName) 
	{
		std::cout << "AttributeDeclartion() called.\n";
	}

	AttributeDeclaration(const AttributeDeclaration& declaration)
	{
		std::cout << "AttributeDeclartion(const AttributeDeclaration& decl) called with this: " << (int)this << " and decl: " << (int)&declaration << ".\n";
	}

	~AttributeDeclaration(void)
	{
		std::cout << "~AttributeDeclartion() called with this: " << (int)this << ".\n";
	}
};

int main(void)
{
	const std::vector<AttributeDeclaration> vFoo =
	{
		{ L"entity" },
		{ L"locked" },
	};

	std::cout << "Initializing vector done.\n";

	return 1;
}
Can you quess what actually causes the issue here? Yeah, its the string-argument in the ctor. The what...? Really, no matter how much I simplified the code, the only thing that causes this is if I put that "const std::wstring&" as first argument. If I remove it, the bug is gone.
 
I said I found the cause of the bug... but obviously not the reason. The string gets correctly constructed (as far as I see) in this code:

basic_string(const _Elem *_Ptr)
: _Mybase()
{	// construct from [_Ptr, <null>)
	_Tidy();
	assign(_Ptr);
}
and the strings value looks OK... it must have been, I've been working with those stored strings quite a while without a problem.
 
So in my example there of course will not be a crash, but you should be able to see the double deletion. So now at least the soure of the problem is determined and you have a very simple code you can run... can anyone make sense out of this behaviour? Is there anything wrong about the way I handle strings here (changed to std::string and removing the L doesn't help), or is this really some compiler issue (again, I'm using VS2013 default compiler)?


Two things I notice about your example code (I don't have MSVC2013 on hand to test initializer lists at the moment):
1. By adding the string to the constructor you disabled the default constructor. Does the bug go away if you have a parameter of a different type? (Try with a POD like int, and a non-POD like, I dunno, std::shared_ptr<int>)
2. You don't have an assignment operator, so the compiler probably generated one for you. Add it in with matching debug output to see if an extra object is being generated and assigned to to match your destructor call. Or add it in as =delete and see if the compiler errors.

Edited by SmkViper, 20 August 2014 - 02:27 PM.





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