CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Results 1 to 15 of 15
  1. #1
    Join Date
    Dec 2007
    Posts
    8

    One of my objects' deconstructors is being called inexplicably

    I created the following class. Whenever the overloaded assignment operator is called, the program seems to deconstruct the object immediately afterwords. Whatever is actually happening, I consistently receive an error message immediately after the program executed the overloaded operator=().

    This is the class:
    Code:
    class key
    {
    public:
    	key(int);//key.length is set to the value passed to the constructor, which allocates the dynamic arrays
    	~key();//The deconstructor deletes the dynamic arrays.
    //More member functions
    	key operator=(const key &rhs);/*This operator sets key.length = rhs.length, deletes and recreates the dynamic arrays, and copies the dynamic arrays in rhs into those in key.*/ //If rhs = key, the function simply executes "return this".
    private:
    	int *keyset;//keyset (key set) is a dynamic array
    	bool *defined_keys;//defined_keys is a dynamic
    	int length;//length is the length of the key set and one more than the highest numbered index of the dynamic array.
    };
    
    key::key(int set_length)
    {//key.length is set to the value passed to the constructor.
    	length = set_length;
    	//Also, create the dynamic arrays.
    	keyset = new int[set_length];
    	defined_keys = new bool[set_length];
    	//Rest of function omitted
    }
    
    key::~key()
    {//The deconstructor deletes the dynamic arrays.
    	delete [] keyset;
    	delete [] defined_keys;
    }
    
    key key::operator=(const key &rhs)
    {//This operator sets this.length = rhs.length, deletes and recreates the dynamic arrays, and copies the dynamic arrays in rhs into those in this.
    	
    	//If rhs == this, the function simply executes "return this" (at the end of the function).
    	//This way, the dynamic arrays are not inadvertently destroyed in a self-assignment.
    	if(this == &rhs)
    	{
    	}
    	else
    	{
    		length = rhs.length;//Set this.length = rhs.length so the dynamic arrays can be correctly recreated
    		
    		//Delete the dynamic arrays so they can have new information put in them
    		delete [] keyset;
    		delete [] defined_keys;
    		
    		//Recreate the dynamic arrays with the new length
    		keyset = new int[length];
    		defined_keys = new bool[length];
    
    		//Put the new information into the arrays
    		for(int i=0;i<length;i++)
    		{//Go through each index and copy the information in rhs to this
    			defined_keys[i] = rhs.defined_keys[i];
    			if(defined_keys[i] == DEFINED)
    			{//Only copy information from rhs.keyset[i] to this.keyset[i] if rhs.keyset[i] is defined.
    //(keyset's members are gradually defined as the program runs, but it is possible to call functions that can access any of its indexes at any time. defined_keys is a parallel array used to make sure that these functions only access defined indexes in keyset.)
    				keyset[i] = rhs.keyset[i];
    			}
    		}
    	}
    	return *this;
    }
    This is a structure that uses the key class:
    Code:
    struct full_set
    {
    	key n,e,d;
    	full_set(int length)//The constructor sets the length of all the keys by calling their constructors. Because of what the program is supposed to do, all the keys have the same lengths.
    	: n(length), e(length), d(length)
    	{}
    	full_set operator=(const full_set &rhs)
    	{//The assignement operator calls the assignment operators for the key objects, which are also overloaded.
    		n.operator=(rhs.n);
    		e.operator=(rhs.e);
    		d.operator=(rhs.d);
    		return *this;
    	}
    };
    This is the declaration of generate_keys, a function I used while testing the overloaded operator=:
    Code:
    full_set generate_keys(int length);
    This is one function I've tried using to test the overloaded operator=:
    Code:
    int main()
    {
    	full_set test(1);
    	test = generate_keys(1);//The program errors out here.
    	cout << test.n.get_value(0) << endl
    		<< test.e.get_value(0) << endl
    		<< test.d.get_value(0);
    	cin.get();
    
    	return 0;
    }
    While test.operator=() runs, there are no errors. After each call of key.operator=() runs, ~key() (the deconstructor) is called. I don't understand why. I'm pretty sure that when n.operator=(rhs.n) is called, n is deconstructed afterwards, and the same thing happens to e and d. Immediately after the program completes test.operator=(), I get the error below.

    I also tested the overloaded operator= with this function:
    Code:
    int main()
    {
    	full_set test(1);
    
    	test.n = generate_keys(1).n;
    	cout << test.n.get_value(0) << endl;
    		//<< test.e.get_value(0) << endl
    		//<< test.d.get_value(0);
    	cin.get();
    
    	return 0;
    }
    In this case, immediately after test.n,operator=() runs, the deconstructor runs and probably deconstructs n. After the deconstructor runs and the program returns to the main function, I get the error.

    This is the error:
    Debug Assertion Failed!

    Program: [location of the executable]
    File: f:\dd\vctools\crt_bld\self_x86\crt\src\dbgdel.cpp [I don't have a drive labeled f: on my computer. ?]
    Line: 52

    Expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)

    For information on how your program can cause an assertion failure, see the Visual C++ documentation on asserts.

    (Press Retry to debug the application)
    From what I've read on other forums, I understand that the _BLOCK_TYPE_IS_VALID(pHead->nBlockUse) error occurs when the program executes the delete command on a pointer that has already been deallocated. I don't understand why the deconstructor has been called at all or why I get this error after the deconstructor is successfully executed.

    I want the program to complete key.operator= by having all the members in the left-hand object have the same values as those in the right-hand object except for the indexes in key.keyset[] that have not been explicitly defined in the right-hand object. For instance, this code:
    Code:
    key a(2),b(3);
    //Program sets b.keyset[0] = 0 and b.keyset[1] = 1 and doesn't set b.keyset[2].
    //In paralell, b.defined_keys[0] and b.defined_keys[1] are set to true to show that the corresponding indexes in b.keyset are defined. b.defined_keys[2] = false.
    a = b;
    Should end with a.length == 3, a.keyset == {0,1,?}, and a.defined_keys == {true,true,false}.

    What is causing the deconstructor to run? What should I do to fix the error and have my program run as I expect it to?

  2. #2
    Join Date
    Aug 2000
    Location
    West Virginia
    Posts
    7,721

    Re: One of my objects' deconstructors is being called inexplicably

    The first thing I noticed is that operator = is returning an object
    by value. You should return by reference.

  3. #3
    Join Date
    Dec 2007
    Posts
    8

    Re: One of my objects' deconstructors is being called inexplicably

    I fixed that now. My key operator looks like this:

    Code:
    class key
    {
    //code
    	key & operator=(const key &rhs);
    //code
    }
    
    key & key::operator=(const key &rhs)
    {
    //code
    return *this;
    }
    I changed full_set as well.

    I didn't know that you could return values by reference. I only knew they could be passed that way. =P

    Thanks for pointing that out.

    I still have a problem. I'm continuing to get the error message, but the deconstructor isn't being called any more.
    Last edited by The Pondermatic; June 17th, 2009 at 06:02 PM.

  4. #4
    Join Date
    Apr 1999
    Posts
    27,449

    Re: One of my objects' deconstructors is being called inexplicably

    Quote Originally Posted by The Pondermatic View Post
    What do you mean? I understand passing by reference vs. passing by value, but I'm pretty rusty with the syntax, and I didn't know you could return by reference.
    Almost every example that you will see where the assignment is overloaded shows returning a reference. So it's strange why you decided to return by value.

    Secondly, if you wrote your assignment operator (and copy constructor, which you didn't show) correctly, why should you care if the object is destroyed? A correct user-defined copy/assignment operator should withstand any and all calls to the destructor. So right there, this is an indication that you didn't write something correctly.

    Third, your assignment operator is faulty in two ways:

    1) Your assignment operator is not exception safe. You call delete[] on the members, and then you call new[]. What if new[] fails? You've deleted the members already, and your object is all messed up.

    The correct way to do this is to first call new[] into temporary pointer variables, do the work on the temp pointers, delete [] the object members, and then assign the temp pointer variables to the object members.
    Code:
    int *temp = new int [whatever];
    bool *temp2 = new bool[whatever];
    //
    //.. do the work using temp and temp2;
    //...
    delete [] keyset;
    delete [] defined_keys;
    keyset = temp;
    defined_keys = temp2;
    //...
    Something like that. If new fails at the beginning, an exception is thrown, and your object is still valid.

    2) Your assignment operator is bogus. Yes, offiicially it is an assignment operator, but this is what's bogus about it:
    Code:
    //Put the new information into the arrays
    		for(int i=0;i<length;i++)
    		{//Go through each index and copy the information in rhs to this
    			defined_keys[i] = rhs.defined_keys[i];
    			if(defined_keys[i] == DEFINED)
    			{//Only copy information from rhs.keyset[i] to this.keyset[i] if rhs.keyset[i] is defined.
    //(keyset's members are gradually defined as the program runs, but it is possible to call functions that can access any of its indexes at any time. defined_keys is a parallel array used to make sure that these functions only access defined indexes in keyset.)
    				keyset[i] = rhs.keyset[i];
    			}
    		}
    See the lines in red? Here is the problem -- an assignment operator is supposed to make an exact copy of the object, not a partial copy. If the object being copied has 3 red dots and 2 polka-dotted elephants, the copied-to object better have 3 red dots and 2 polka-dotted elephants.

    In other words, there should be no check for anything -- you should just copy what's there, regardless of what the object being copied contains. Once you start being too fancy, you end up with two objects, where one is not a real copy of the other. That's why I say that your copying is bogus and faulty.

    If you have two objects, A and B, and B is a copy of A, then wherever I use an A object, if I instead use a B object, the program must behave exactly the same way. Many hard to find bugs originate from assignment operators that try to fool around and not make real copies.

    Finally, much of this code is not necessary at all if you used std::vector<int> instead of int*, and (in this case) std:eque<bool> instead of bool*. Any reason why you didn't use these standard C++ container classes, thus eliminating the need for copy/assignment operators to be written?

    The advice may not fix your problem, but you have issues that are just as, if not more important than having the destructor called.

    Regards,

    Paul McKenzie
    Last edited by Paul McKenzie; June 17th, 2009 at 06:16 PM.

  5. #5
    Join Date
    Dec 2007
    Posts
    8

    Re: One of my objects' deconstructors is being called inexplicably

    I took your advice. Now I have this:

    Code:
    key& key::operator=(const key &rhs)
    {//This operator sets this.length = rhs.length, deletes and recreates the dynamic arrays, and copies the dynamic arrays in rhs into those in
    //this.
    	
    	//If rhs == this, the function simply executes "return this" (at the end of the function).
    	//This way, the dynamic arrays are not inadvertantly destroyed in a self-assignemnt.
    	if(this == &rhs)
    	{
    	}
    	else
    	{
    		length = rhs.length;//Set this.length = rhs.length so the dynamic arrays can be correctly recreated
    		
    		//Delete the dynamic arrays so they can have new information put in them
    		delete [] keyset;
    		delete [] defined_keys;
    		
    		//Recreate the dynamic arrays with the new length
    		{
    			int *temp_keyset = new int[length];//temp_keyset copies keyset from rhs, then gives the address it points to to this.keyset
    			for(int i = 0;i < length;i++)
    			{//Copy rhs.keyset to temp_keyset
    				temp_keyset[i] = rhs.keyset[i];
    			}
    			keyset = temp_keyset;//Give temp_keyset's address to this.keyset.
    		}
    		{
    			bool *temp_defined_keys = new bool[length];//temp_defined_keys copies defined_keys from rhs, then gives the address it points
    														//to to this.defined_keys
    			for(int i = 0;i < length;i++)
    			{//Copy rhs.defined_keys to temp_defined_keys
    				temp_defined_keys[i] = rhs.defined_keys[i];
    			}
    			defined_keys = temp_defined_keys;//Give temp_defined_keys's address to this.defined_keys.
    		}
    	}
    	return *this;
    }
    I still get the error, which is FTL. =P Thanks for the convention advice.

    What should I do to fix the error?

  6. #6
    Join Date
    Apr 1999
    Posts
    27,449

    Re: One of my objects' deconstructors is being called inexplicably

    My last post should also point out that the only reason for writing copy/assignment operators that don't seem to do exact copying are for the cases where you are doing reference-counting to copy objects.

    Still, even in that scenario, the relationship holds for objects A and B. If B is a copy of A, the program must behave exactly the same way if either A or B is used.

    The rationale for all of this is that assignment is an operation that need not be called explicitly by the programmer. The compiler can call assignments (and copy construction) by passing or returning by value, casting, assigning to containers that have copy semantics, etc. So by coding fake assignments, your opening yourself up to bugs you never expected since the assignment is being called in unexpected places.

    In other words, in most cases, you don't have total control of when assignment is done. If you tried to micromanage when assignments are done, you will wind up wasting your time, since the compiler can optimize (or not optimize) away assignments.

    Regards,

    Paul McKenzie

  7. #7
    Join Date
    Apr 1999
    Posts
    27,449

    Re: One of my objects' deconstructors is being called inexplicably

    Quote Originally Posted by The Pondermatic View Post
    I still get the error, which is FTL. =P Thanks for the convention advice.

    What should I do to fix the error?
    We need a real program that actually duplicates the error. We don't know when, where, or how that code is actually being called to produce the error.

    Regards,

    Paul McKenzie

  8. #8
    Join Date
    Apr 1999
    Posts
    27,449

    Re: One of my objects' deconstructors is being called inexplicably

    Code:
    		//Delete the dynamic arrays so they can have new information put in them
    		delete [] keyset;
    		delete [] defined_keys;
    This is still wrong. You're deleting your members before you've called new. Again, what would happen if new[] fails, and you did this? You would have a messed up object.

    Regards,

    Paul McKenzie

  9. #9
    Join Date
    Dec 2007
    Posts
    8

    Re: One of my objects' deconstructors is being called inexplicably

    I guess that makes sense. =P

    The error still isn't going away. To see whether or not it had anything to do with the operators, I commented all the overloaded assignment operators out and still got the error. So now, I know I'm getting the error neither because of my overloaded operators (which I could have made better in the first place >_<) nor because of the deconstructors.

    Edit: Ninja'd. =P
    Last edited by The Pondermatic; June 17th, 2009 at 06:48 PM.

  10. #10
    Join Date
    Apr 1999
    Posts
    27,449

    Re: One of my objects' deconstructors is being called inexplicably

    BTW, this version of your class does all of the work you're doing now, without any extra coding:
    Code:
    #include <vector>
    #include <deque>
    
    class key
    {
    public:
    	key(int);
            std::vector<int> keyset;
    	std::deque<bool> defined_keys;
            int  length() const { return static_cast<int>(keyset.size()); }
    };
    That's it, nothing else. No destructor, no assignment operator, etc. is necessary. Not only is it easier to maintain, it works without any intervention from the coder at all.

    Regards,

    Paul McKenzie

  11. #11
    Join Date
    Apr 2007
    Location
    Mars NASA Station
    Posts
    1,436

    Re: One of my objects' deconstructors is being called inexplicably

    In cases, you need to learn how to write assignment operator. I strongly advised you to change your if statement.

    Code:
    if (this !=&rhs)
    {
    }
    And follow the Paul approach new first before delete.
    Thanks for your help.

  12. #12
    Join Date
    Dec 2007
    Posts
    8

    Re: One of my objects' deconstructors is being called inexplicably

    I found a way to stop the error, but it's totally a hack. If I comment out the deconstructor , the error goes away. I know that's horrible practice, though. What kind of mistake does the faulty deconstructor suggest? Is it worth it to leave the deconstructor out, or would I be committing a gigantic faux paus?

    EDIT: You guys are really hammering me for my style. I am, admittedly, a newb, so any advice you guys give me is great. =) I would like to focus some more on the error, though. =P How badly do you want to see my whole program to help find the error?
    Last edited by The Pondermatic; June 17th, 2009 at 08:46 PM.

  13. #13
    Join Date
    Apr 2004
    Location
    Canada
    Posts
    1,342

    Re: One of my objects' deconstructors is being called inexplicably

    Quote Originally Posted by The Pondermatic View Post
    I found a way to stop the error, but it's totally a hack. If I comment out the deconstructor , the error goes away. I know that's horrible practice, though. What kind of mistake does the faulty deconstructor suggest? Is it worth it to leave the deconstructor out, or would I be committing a gigantic faux paus?
    You definitely need a destructor (not "deconstructor" ) if you allocate dynamic memory in the constructor.

    If you're still getting an error (without commenting out the destructor), post a complete piece of code that reproduces the error.
    Old Unix programmers never die, they just mv to /dev/null

  14. #14
    Join Date
    May 2002
    Location
    Lindenhurst, NY
    Posts
    867

    Re: One of my objects' deconstructors is being called inexplicably

    Quote Originally Posted by The Pondermatic View Post
    How badly do you want to see my whole program to help find the error?
    Don't just dump your entire program. Put in some effort to create an SSCCE and post that.

  15. #15
    Join Date
    Aug 2000
    Location
    West Virginia
    Posts
    7,721

    Re: One of my objects' deconstructors is being called inexplicably

    As mentioned, you should post your current code and test program.

    Also, have you implemented a copy constructor ? I don't see one
    in any of your posts.

    The general rule is :

    If you need to code any one of : assignment operator , destructor ,
    copy constructor, then you probably need to code all three of them.

    Often, the assignment operator can be coded using the copy constructor.

Tags for this Thread

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  





Click Here to Expand Forum to Full Width

Featured