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

Started by
19 comments, last by Zahlman 16 years, 2 months ago
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).
Advertisement
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"
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?
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.
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.


newmember.~myclass(); <-- you are explicitly calling the destructor ?

how many times is the destructor going to be called ?
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.
...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.
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.
NextWar: The Quest for Earth available now for Windows Phone 7.
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;

This topic is closed to new replies.

Advertisement