4

A simple question here, I have several classes in my code, but only one of them exhibits this issue and I cannot for the life of me work out why. When I create an instance of the class, the destructor is called straight after, yet the instance of the class does not actually appear to be deleted.

Maybe I could live with that if there were not delete[] operations in the destructor that DO affect the instance of the class.

I read somewhere about the 'rule of three' or something, so attempted to see what I was missing. I already have a default constructor as well as a user-defined one. I then added what I think is called a copy-constructor, something like this:

MyClass::MyClass(const MyClass &duplicate)
{
    variable1 = duplicate.variable1;
    variable2 = duplicate.variable2;
    // etc
}

What am I missing here that could cause this issue?

EDIT: The requested code. I've renamed everything so that it's all clear (this code still compiles with the issue). First, the header file, MyClass.h:

#ifndef MYCLASS_H
#define MYCLASS_H
#ifndef UNICODE
#define UNICODE
#endif
#include <string>

class MyClass
{
public:
    MyClass();
    MyClass::MyClass(const MyClass &);
    MyClass(int, std::wstring inputWord, int);
    ~MyClass();

    int intOne;
    int intTwo;
};
#endif

Next MyClass.cpp:

#include "MyClass.h"
#include <Windows.h>


MyClass::MyClass(const MyClass &duplicate)
{
    intOne = duplicate.intOne;
    intTwo = duplicate.intTwo;
}
MyClass::MyClass()
{
}

MyClass::~MyClass()
{
    MessageBox(NULL, TEXT("TEST"), TEXT("TEST"),0);
}

MyClass::MyClass(int intOneInput, std::wstring stringInput, int intTwoInput)
{
    intOne = intOneInput;
    intTwo = intTwoInput;
}

And finally how I'm creating my object:

MyClass test(0, TEXT("TEST"), 0);

[Copied from op's comment]

Actually, scratch my last comment, the deconstructor is NOT called with that particular line (until it goes out of scope), the line that does is words.push_back(MyClass(0, TEXT("TEST"), 0)); declared as std::vector<MyClass> words

young
  • 2,163
  • 12
  • 19
Interminable
  • 1,338
  • 3
  • 21
  • 52
  • 2
    The problem isn't in the code that you have shared, somewhere you're creating a temporary. We'll need to see that code. – johnsyweb Apr 30 '12 at 22:00
  • 1
    We'll need to see the definition of your class (constructors, destructor, member variables) and the usage of your class. – Tim Apr 30 '12 at 22:00
  • The code above is okay. You maybe should point out where exactly the problem is happening, and how you prove that it is happening (e.g. printf in destructor?) Maybe your problem is with scopes of variables, but I can hardly tell unless you post the failing code. – user826955 Apr 30 '12 at 22:02
  • Are you returning it as value? – victor.t Apr 30 '12 at 22:02
  • I have added the requested code. – Interminable Apr 30 '12 at 22:19
  • @Interminable you were speaking about pointers held by your class. Also what make you think the memory is released (delete[]). What error do you get ? – log0 Apr 30 '12 at 22:25
  • The delete[] is not causing the issue. The issue remains with the code pasted above. Literally, all the code I have pasted above is what causes the issue for me. I removed everything else to try and narrow down the cause. All I do with the class is create an instance of it, which is where the issue reveals itself (the deconstructor is called immediately). I can see this by the fact that the MessageBox I have in the deconstructor appears the instant I step over the line of code that creates the class instance. – Interminable Apr 30 '12 at 22:27
  • 1
    You're still missing something. How do you conclude that the destructor is being called while the object still exists? What code surrounds the one-liner that you've shown us that creates the object? – Mark Ransom Apr 30 '12 at 22:28
  • Because I can step in to it in the debugger, but if I pause execution straight after the line creating the instance, I can VIEW the values stored in my variables. I use VS2010 as my IDE. – Interminable Apr 30 '12 at 22:28
  • @Interminable Don't trust the debugger to reliably tell you the sequence in which events occur. If you've posted this much, why not show us the rest of `main`? ;v) – Potatoswatter Apr 30 '12 at 22:54
  • There's not much in `WinMain`, but there is quite a bit of code in my other source files, mostly Window.cpp. It's a Windows application. And the debugger hasn't let me down so far (ever) in terms of the sequence of code execution. I'll add that I have also proceeded to manipulate the values stored in my class instance to prove they're there and not just 'ghosts' of former values. – Interminable Apr 30 '12 at 23:03
  • @Interminable you missed that you are using vector. That's core information to you question. – young Apr 30 '12 at 23:13
  • Yeah, sorry about that. Thing is, I commented it out and was doing it as I described, but I was going out of scope. However the vector thing does appear to be at the root of the problem. In the test file I created, I now get the following output: `ctor 002DF9C0 ctor 00B44148 dtor 002DF9C0 TEST dtor 00B44148 Press any key to continue . . .` With this code: `int main() { std::vector words; words.push_back(MyClass(0, TEXT("TEST"), 0)); std::cout<<"TEST"< – Interminable Apr 30 '12 at 23:26
  • @Interminable: according to your last comment, what's happening (as several people have mentioned) is that a temporary is being created and copied into the vector. The first destructor call is for the temporary. So, is your question answered? – Michael Burr May 01 '12 at 00:20

4 Answers4

2

The destructor should be called when the object is destroyed. If you create the object with new, The destructor will be called when you call delete on the object. Otherwise it should be called when it goes out of its scope. You can set breakpoint inside its destructor and see the callstack to check what is calling the destructor. Hope it helps.

[Update] Try to add the below printf for all ctor and dtor to make it sure you are not confused with temporarily created objects.

printf("ctor %p\n", this); // in constructors

printf("dtor %p\n", this); // in destructor

[Update]

words.push_back(MyClass(0, TEXT("TEST"), 0));

This creates a temporary object as stl containers(like vector) always "copy" things to store. (Unless "move" happens. I don't want to start explaining "move" and rvalue here.)

young
  • 2,163
  • 12
  • 19
  • Add the assignment operator and move constructor to make sure you get them all. – Martin York Apr 30 '12 at 22:44
  • @young, in order to try your code I had to create a new project with the above class (for the CMD window for printf), and it functioned as it should do. So something must be causing issues, but NOTHING else is interacting with this class...I have no idea what could be causing the problem I'm having. – Interminable Apr 30 '12 at 22:45
  • @Loki, An assignment operator and a move constructor you say? I'll have to look up those. – Interminable Apr 30 '12 at 22:47
  • @Interminable: Assignment operator: `MyClass& operator=(MyClass& copy)` Move semantics don't bother yet you have a lot more to learn before you get there. – Martin York Apr 30 '12 at 22:48
  • @Interminable try to use OutputDebugString with sprintf. – young Apr 30 '12 at 22:50
  • Actually, scratch my last comment, the deconstructor is NOT called with that particular line (until it goes out of scope), the line that does is `words.push_back(MyClass(0, TEXT("TEST"), 0));` declared as `std::vector words`; – Interminable Apr 30 '12 at 22:54
  • sprintf doesn't seem to like 'this' (`cannot convert parameter 2 from 'MyClass *const' to 'const char *'`) – Interminable Apr 30 '12 at 23:08
  • 1
    @Interminable see my update. You are creating a temp obj which is destroyed right after its copy is stored in the vector. – young Apr 30 '12 at 23:11
  • It definitely looks that way. So how do I work around this copying issue? – Interminable May 01 '12 at 00:42
  • I have worked around the issue by having a boolean variable that sets itself to true if the copy constructor is called. If this variable is set to true, certain things in the deconstructor do not execute (such as delete[], as new[] has not yet been used). I'm going to mark this as the right answer for pointing out the temporary objects issue, thanks. – Interminable May 01 '12 at 00:59
  • 1
    @Interminable: you're taking a dangerous step with that design for a copy constructor. What happens to the copies that occur inside the `vector` if/when it moves things around? You need to either move away from using raw pointers (that may be easy or hard depending on what you're handling with raw pointers right now) or you need to properly implement a copy constructor and an assignment operator for this class. Search for "C++ rule of three" for more details. Based on the comment, your class is likely seriously broken until you make those changes. – Michael Burr May 01 '12 at 01:30
  • Yeah, my way seems to be somewhat flawed when I add more than one item to the vector, probably because stuff gets moved around... – Interminable May 01 '12 at 13:57
1

A possible issue (but we need to see more) is that your copy constructor perform a shadow copy and you construct your object copy-constructing from a temporary. When the temporary get destroyed, the memory held by the temporary is released and the new object now has pointer on deleted memory.

One solution is: Perform a deep copy when copy constructing.
http://www.learncpp.com/cpp-tutorial/912-shallow-vs-deep-copying/

Another is: Use smart pointers instead of raw pointers.
What is a smart pointer and when should I use one?

Community
  • 1
  • 1
log0
  • 10,489
  • 4
  • 28
  • 62
  • -1: How is deep copy or even smart pointers relevant to this question. There are no internal pointers so deep copy is not relevant. We are declaring an automatic object so smart pointers are not relevant. – Martin York Apr 30 '12 at 22:47
  • @Ugo, I am finding the deep copy stuff very interesting AND relevant for what I am trying to do (although not directly related to this question). I am trying to store a wchar_t array (unlike the example's char array), but I seem to be running in to an issue. Whenever I try to `resize()` my vector for a second class instance, it appears to erase the array text in the entry. – Interminable May 01 '12 at 01:55
  • I've managed to fix my issue, I needed to have both an assignment operator and a copy constructor even though the code for both was very similar. Thanks! – Interminable May 01 '12 at 14:48
1

This:

MyClass test(0, TEXT("TEST"), 0);

declares an automatic object.
The object is destroyed when it goes out of scope.

{
   MyClass test(0, TEXT("TEST"), 0);   // constructed here
}                                      // destructed here.
Martin York
  • 257,169
  • 86
  • 333
  • 562
  • I am aware of this. The deconstructor is being called BEFORE it goes out of scope. The object itself however does not appear to be destroyed after this has happened. – Interminable Apr 30 '12 at 22:50
  • 2
    @Interminable: No it is not. I can guarantee that. More likely is you are not showing us something and a temporary (you don't expect) is being created. **A compilable example** that illustrates the problem would be the preferable question. Sort of showing us half the problem is not helping as you can only get a guess as the answer. – Martin York Apr 30 '12 at 22:52
1

I know this is an old thread but if it can help someone in the future, I've seen the same issue testing the code in this post when copy constructor is not defined: https://www.viva64.com/en/w/v690/:

1 raise                                    0x7ffff4e320e0
2 abort                                    0x7ffff4e336c1
3 __libc_message                           0x7ffff4e75427
4 malloc_printerr                          0x7ffff4e7bc43
5 MyArray::Clear         mainwindow.cpp 50 0x4204a5
6 MyArray::~MyArray      mainwindow.cpp 53 0x4204e2
7 MainWindow::MainWindow mainwindow.cpp 73 0x41416b
8 main                   main.cpp       10 0x40c372

And I see in your code the copy constructor is defined the header with:

MyClass::MyClass(const MyClass &);

Instead of simply:

MyClass(const MyClass &);

The I'm not sure your copy constructor is properly declared and used.

Anyway with current GCC version this particular case should not happen since it should not pass because of "extra qualification ‘MyArray::’ on member ‘MyArray [-fpermissive]" error, but it can help people for which copy constructor is missing.

gluttony
  • 402
  • 6
  • 14