Sign in to follow this  

[Resolved] (C++) deconstructor causing access violation in odd location

This topic is 3598 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

I have a class which has these members:
std::map<const char*,int> section_list;
std::vector<std::map<const char*,int>> section_key;
std::vector<char*> str_vals;
To summarize how they're used (which will explain an oddity the deconstructor), two strings can be used to return a single value. section_list maps a string to the location of a map in section_key. The maps in section_key map a second string to the location of a value in str_vals. To destroy this, I use:
for(unsigned int i=0;i<section_key.size();i++)
  section_key.at(i).clear();
section_list.clear();
section_key.clear();
str_vals.clear();
Which appears to clean up the used memory quite nicely, however once the program terminates, I receive an access violation. Even when I experimented by using a default destructor, the access violation still occurs. The "just in time" debugger points me to _Erase(_Root()); as the break point, though I don't know if that information is at all useful. I've little to no skill at debugging when it involves code I didn't personally write. [Edited by - Zouflain on February 8, 2008 5:26:02 AM]

Share this post


Link to post
Share on other sites
Quote:
Original post by Zouflain
The "just in time" debugger points me to _Erase(_Root()); as the break point, though I don't know if that information is at all useful. I've little to no skill at debugging when it involves code I didn't personally write.

(Assuming you're using visual studio) If you look at the call-stack window when this happens, you should be able to step back through the stack until you get back to your own code.

[edit]
Also, adding "myMemberSTLContainer.clear();" into your destructor isn't actually needed. Your destructor will automatically call the destructors of all of your members, which in this case will free the memory used by your maps/vectors automatically.

Share this post


Link to post
Share on other sites
Thank you for the information. I wanted to be sure that the vector containing maps was completely cleared out, rather than form a memory leak by having maps without having been .clear()'ed. If this isn't necessary, that's relieving. I was certain this would lead to a memory leak.

As for what you said about debugging, however, I'm mostly drawing a blank. I am using Visual Studio, but usually I debug a program by studying my own work long enough to figure out where the potential buffer overflows are (or whatever the case may be) and correcting them that way. By call-stack I'll assume you mean the program's disassembly, and the specific line that generates the break point reads
00416679  mov         eax,dword ptr [eax]
I recognize it as assembly, but I've never studied it enough to understand this specific call.

As an aside, I should point out that I'm certain that this happens after all my code goes through, because I ran a bit of a test:
int main(){
various_operations()
printf("Other operations successful...\n");
system("PAUSE");
mycode();//including the destructor
printf("My operations successful...\n");
system("PAUSE");
return(0);
}
I get through the last system("PAUSE") without issue but right before the program completely terminates, the debugger alerts me to an access violation. I don't really get how this works; is windows trying to erase data that was already previously erased? And how can I prevent that?

The program is SDL based, by the way, and not platform dependent. I just happen to have compiled it in windows and Visual Studio.

Share this post


Link to post
Share on other sites
The char* and const char* template parameters alarm me.

How do you fill your map?


Also i often see global static maps causing problems. Are they global? If so try to encapsulate them somewhere to make sure they're getting destroyed earlier on.

Share this post


Link to post
Share on other sites
I see char*'s without any deletes, and that makes me worry. It's very likely you're leaking memory at the moment. Clearing a container of pointers only deletes the pointers themselves, but not what they point to. You still need to loop over the container and delete each pointer element, before clearing (or destructing).

If you're already deleting the pointers in those containers elsewhere, then disregard my post :)

Share this post


Link to post
Share on other sites
Any chance, you are using visual studio 6.0? I ran into a problem with std::map in 6.0's implementation once that caused deconstruction access violations after program completion as well. I spent some time banging my head on it before I decided to refactor around the map...either choosing a different structure for that job or putting pointers in it instead of objects. I really cant remember, but I do recall explicitly finding other people complaining about bugs in std::map for that release, which I attributed to my problem.

Sorry, not many more details, thats all I remember.

Share this post


Link to post
Share on other sites
I have a very limited ammount of globals, and these are things that really need to be the same throughout the program

--a boolean that must be true else the gameloop terminates (any point in program can kill game this way)
--The time (in ticks) at which the game started
--The time (in ticks) of the last frame
--The current number of ticks elapsed
--A member of this class (perhaps this is the issue?)
--A vector storing a set of objects, all derived from a generic base class with a few virtual functions (useful because I can use it to reference every active object which can in turn react to an event).

I did forget to destroy the vector, but as it's always in use until the game is over, I didn't see it as a priority (and, for good measure, I've just now .clear()'ed it in the cleanup. The global member of the class is because this is used to map control settings, and it is destroyed in the mycode() segment of the above test I ran without issue.

As for the char*... yeah, it's automatic for me to use cstrings rather than strings. Bad habit, which I'll fix, but first I need to find the problem here. Does that appear to be a part of the problem? It's undoubtedly possible.

Edit:

std::map<const char*,int>::iterator mapit;
mapit = section_list.begin();
for(;mapit!=section_list.end();++mapit){
delete mapit->first;
}
for(unsigned int i=0;i<section_key.size();i++){
mapit = section_key.at(i).begin();
for(;mapit!=section_key.at(i).end();++mapit)
delete mapit->first;
section_key.at(i).clear();
}
for(int j=0;j<str_vals.size();j++)
delete str_vals.at(j);
section_list.clear();
section_key.clear();
str_vals.clear();
Cleaner destructor. This one clears out those char*... rather sad that I [u]completely[/u] forgot about them. Very poor practice, but that's why I'm doing what I'm doing; live and learn.

Edit (2):
A side note; I copied this into a function that wasn't declared as a deconstructor - no issue. Having just a blank deconstructor still generated the issue. I'm using Visual Studio 8.0.5 (a few trailing digits), if that's any help. I have gcc and MinGW but my linux is out of order anyway.

[Edited by - Zouflain on February 8, 2008 2:46:45 AM]

Share this post


Link to post
Share on other sites
You have a leak, invalid pointer or buffer overrun somewhere in you code. The callstack usually sits in the lower right window when you debug, and will show you the series of calls made. You can double-click in it to step to the point in the code, and that way backtrack to the point where it really crashes.

Share this post


Link to post
Share on other sites
Quote:
std::_Tree<std::_Tmap_traits<char const *,int,std::less<char const *>,std::allocator<std::pair<char const * const,int> >,0> >::begin()
From call stack, and this links me over to xtree. Even though it's something I really ought to know, I'm unfamiliar with the deconstructor for a map so it would be difficult to back trace any further. No call I make to begin() throws a violation (again, this is happening as the program closes, after all my code has executed and a system pause has interrupted things just to be sure). The only code after system("PAUSE") is return(0);

Share this post


Link to post
Share on other sites
Quote:
Original post by Zouflain

std::map<const char*,int>::iterator mapit;
mapit = section_list.begin();
for(;mapit!=section_list.end();++mapit){
delete mapit->first;
}
This much sends alarm bells ringing for me!
You really really must not use a char* as a map key.

Share this post


Link to post
Share on other sites
Quote:
Original post by iMalc
Quote:
Original post by Zouflain

std::map<const char*,int>::iterator mapit;
mapit = section_list.begin();
for(;mapit!=section_list.end();++mapit){
delete mapit->first;
}
This much sends alarm bells ringing for me!
You really really must not use a char* as a map key.


I plan on removing any refference to char* and instead use a string, but that doesn't effect the issue. Deconstructing this (even with a blank deconstructor, or using the above deconstructor in a non-deconstructive function and then a blank deconstructor) causes the program to crash just before terminating. Using the above deconstructor in a normal function causes no issue. Again, everything works until the main() function tries to return(0); if I use a deconstructor (blank, full, or otherwise), then it crashes (not a very major crash, all things considered, but I would like to know why it's doing it).

Share this post


Link to post
Share on other sites
A problem with c++ is that things can get corrupted in memory and the problems only show up later in what appears to be perfectly valid code.

in windows - try to validate the heap in your function that calls the clear() on the objects both before and afterward.

_heapchk();
myset.clear();
_heapchk();

this is a much stronger approach for intractable memory problems than using the debugger which may break in a place that is unrelated to the cause of the problem.

Also you do realise that the map< char *, ... > is ordering by pointer value and not actually doing any string type comparison ? ie two strings with the same content are not going to be equal or comparable unless you have taken steps to make them globally unique.
ie "mystring" != "mystring"

Share this post


Link to post
Share on other sites
Quote:
Original post by iMalc
Quote:
Original post by Zouflain

std::map<const char*,int>::iterator mapit;
mapit = section_list.begin();
for(;mapit!=section_list.end();++mapit){
delete mapit->first;
}
This much sends alarm bells ringing for me!
You really really must not use a char* as a map key.
That sends alarm bells ringing for me too. Because you can't modify the keys of a map when they're still in the map without completely breaking it.

If you need to change the keys ("first" of the pairs), you need to remove the value from the map, change the key and remove it. Even if you're doing that in the destructor, it looks extremely suspect.

Why aren't you using std::string?

Share this post


Link to post
Share on other sites
Indeed "astring"!="astring", but strcmp("mystring","mystring") will determine if the strings are the same. It's archaic, but then so are cstrings.

As for _heapchk(), it returns a -2. This is before loading the class, afterwards, and after the .clear() function (the name I've given the non destructor function). No idea what that means though. I even get it after using deconstructor - it doesn't seem to change at any point during the program's execution.

Note: the maps aren't changed there. The value being pointed-to by the key is being changed (rather, deleted). The map itself is cleared all at once with the following call section_key.at(i).clear(). Unless I've made some horrid mistake. Even so, the program still has the error when none of the code I've posted appears.
#include<vector>
#include <map>

class myclass{
std::map<const char*,int> section_list;
std::vector<std::map<const char*,int>> section_key;
std::vector<char*> str_vals;
};
int main(){
myclass newmember;
newmember.~myclass();
return(0);
}
Compiles... runs... crashes. I really, really don't understand it.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zouflain
Indeed "astring"!="astring", but strcmp("mystring","mystring") will determine if the strings are the same. It's archaic, but then so are cstrings.
Depends on how the strings are allocated. If they're compile-time constants, there's a good change the pointers will be equal.

Quote:
Original post by Zouflain
Compiles... runs... crashes. I really, really don't understand it.
Why are you suing C strings? They probably don't sort in the std::map the way you expect them to (Since operator< compares pointer values for C-strings), and you need to manage the memory allocation for them. If you're using std::map already, there's absolutely no reason not to use std::strings.

I wouldn't be in the least bit surprised if this bug was down to mis-using char*'s, e.g. double deleting them or something.

Share this post


Link to post
Share on other sites
Quote:
Original post by chairthrower
newmember.~myclass(); <-- you are explicitly calling the destructor ?

how many times is the destructor going to be called ?
D'oh - I missed that [smile]

As chairthrower pointed out, you shouldn't ever explicitly call the destructor (Unless you're doing low level memory managment). In your code, it'll be called twice; once when you call it, and once when the object goes out of scope. The std::map is being cleared out twice, which is causing the crash.

Share this post


Link to post
Share on other sites
...well. Isn't that funny?

I've never actually tried to directly call a destructor, and apparently that's the problem. Little things (that turn out to be big things) like this are good learning experiences at least. The real lesson is don't trust code you wrote at 9:00AM when you started before 11:00PM... I have no idea what I thought directly calling the destructor was legal. But, after having destroyed it myself, windows tries to destroy it and there's the exception I'd imagine.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zouflain
As for what you said about debugging, however, I'm mostly drawing a blank. I am using Visual Studio, but usually I debug a program by studying my own work long enough to figure out where the potential buffer overflows are (or whatever the case may be) and correcting them that way.

Visual Studio comes with an extremely powerful debugger. One of the best that you can get, in fact. You should learn to use it as soon as you can.

Quote:

By call-stack I'll assume you mean the program's disassembly, and the specific line that generates the break point reads
00416679  mov         eax,dword ptr [eax]
I recognize it as assembly, but I've never studied it enough to understand this specific call.

No, that's not the call stack. The call stack should be near the bottom right corner of the screen when you run the program from VS. A call stack basically tells you what function called what. For example, the program might break inside a function called Foo(). The call stack could tell me that the function Bar() was the one that called Foo(), and that the function main() was the one that called Bar() in the first place. This is extremely useful in determining where, exactly, the error occurred.

A more practical example is if the program breaks inside std::vector somewhere. To me, that's not very useful, since any portion of my code from anywhere could have made that call to std::vector... and so I have no idea what portion of my code is broken. Using the stack trace I can step backwards in the program, to find the function that I wrote which made the offending call which caused std::vector to break.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zouflain
Indeed "astring"!="astring", but strcmp("mystring","mystring") will determine if the strings are the same. It's archaic, but then so are cstrings.


the problem is that map<> or set<> does not know what your intention is with respect to ordering the strings. If you actually want to compare in the way you indicate then you would need to pass the strcmp function explicitly as the comparator.
std::set< const char *,
boost::function< bool ( const char *, const char *) > >
z( lambda::bind( &strcmp, _1, _2) < 0 ) ;

z.insert( "bananna" );
z.insert( "orange");
z.insert( "apple" );
for_each( z.begin(), z.end(), cout << _1 << '\n' );

if you use std::string then the default comparator std::less will automatically work in the way that you expect. what it buys you is simplicity. std::string's are easy and work well with char * - i use char * as well in places but you can convert freely between the two
string  s0;
const char *s1 = s0.c_str();
string s2 = s1;

Share this post


Link to post
Share on other sites
Quote:
Original post by Zouflain
Indeed "astring"!="astring", but strcmp("mystring","mystring") will determine if the strings are the same. It's archaic, but then so are cstrings.


The thing is, std::map doesn't know about strcmp (you can tell it, but it's still a bad idea). And it needs to be able to compare your keys in order to sort its internal data structure, and provide proper "map-like" behaviour.

The reason you can get to the last breakpoint before the end of main(), but not past it, is that the crash happens during deconstruction.

Quote:
The value being pointed-to by the key is being changed (rather, deleted).


Was it allocated via new? If not, don't delete it.

Do you have a mix and match? Then you're screwed. You can't reliably find out, given only a pointer, how the pointed-at data was allocated.

Quote:
#include<vector>
#include <map>

class myclass{
std::map<const char*,int> section_list;
std::vector<std::map<const char*,int>> section_key;
std::vector<char*> str_vals;
};
int main(){
myclass newmember;
newmember.~myclass();
return(0);
}
Compiles... runs... crashes. I really, really don't understand it.


That's much worse. Only experts who are doing very specific things ("implementing std::vector from scratch for a brand new compiler" is one of them) who know what they are doing should ever call a destructor explicitly.

Also, there is no need or reason to put parentheses around a return value. 'return' is not a function; it's a keyword.




Anyway. Just using std::string is much easier than trying to figure out exactly why not using it isn't working. :)

For what it's worth, the code at the break point:


00416679 mov eax,dword ptr [eax]


is simply a pointer dereference.

Share this post


Link to post
Share on other sites

This topic is 3598 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