Delete[] within destructor, allocating within constructor

Question

I allocate memory in constructor and then do delete[] in destructor but I get _Block_Type_Is_Valid (pHead->nBlockUse)" Error. I came across the rule of three and got along with that. In my program I just have one instance and I am using nor Copy Constructor neither Copy Assignment Operator so I thought that I just need explicitly declared destructor. In constructor I am copying the values of char array and I cant get the idea what is wrong. Here is short listing of program code. Header:

class CTest
{
private:
    char *TestTable;
    int TestTableLength;
    std::chrono::steady_clock::time_point StartPoint;
    std::chrono::steady_clock::time_point EndPoint;
    std::chrono::steady_clock::time_point CheckPoint;
    std::chrono::system_clock::duration d;
public:
    CTest(char *SignTable);
    ~CTest();
    void NewCombination();
    bool AskToPlay();
    bool AskForNewCombination();
    void Play();
};

Source:

CTest::CTest(char *SignTable)
{
    int SignTableLength = 0;
    for (int i = 0; *(SignTable + i) != '\0'; i++)
    {
        SignTableLength++;
    }
    TestTableLength = SignTableLength ;
    TestTable = new char[TestTableLength];
    for (int i = 0; *(SignTable + i) != '\0'; i++)
    {
        *(TestTable + i) = *(SignTable + i);
    }
}


CTest::~CTest()
{
    delete[] TestTable;
}

void CTest::NewCombination()
{
    int tmpInt; 
    char tmpChar;
    if (TestTable != NULL)
    {
        for (int i = 0; i < TestTableLength - 1; i++)
        {
            tmpInt = rand() % (TestTableLength - i);
            tmpChar = TestTable[TestTableLength - 1 - i];
            TestTable[TestTableLength - 1 - i] = TestTable[tmpInt];
            TestTable[tmpInt] = tmpChar;
        }
    }
}

bool CTest::AskToPlay()
{
    char play;
    std::cout << "Do you want to play again ?" << std::endl << "If yes press 'y' else  press something else" << std::endl;
    std::cin >> play;
    if (play == 'y') return true;
    else return false;
};

bool CTest::AskForNewCombination()
{
    char newcom;
    std::cout << "Do you want me to create new combination?" << std::endl << "If yes press 'y' else press something else" << std::endl;
    std::cin >> newcom;
    if (newcom == 'y') return true;
    else return false;
};

void CTest::Play()
{
    StartPoint = std::chrono::steady_clock::now();
    CheckPoint = std::chrono::steady_clock::now();
    std::cout << "3\t";
    d = CheckPoint - StartPoint;
    while (std::chrono::duration_cast<std::chrono::milliseconds>(d).count() < 1000)
    {
        CheckPoint = std::chrono::steady_clock::now();
        d = CheckPoint - StartPoint;
    }
    std::cout << "2\t";
    while (std::chrono::duration_cast<std::chrono::milliseconds>(d).count() < 2000)
    {
        CheckPoint = std::chrono::steady_clock::now();
        d = CheckPoint - StartPoint;
    }
    std::cout << "1" << std::endl;
    while (std::chrono::duration_cast<std::chrono::milliseconds>(d).count() < 3000)
    {
        CheckPoint = std::chrono::steady_clock::now();
        d = CheckPoint - StartPoint;
    }
    std::cout << "START!" << std::endl;
    StartPoint = std::chrono::steady_clock::now();
    for (int i = 0; i < (TestTableLength ) ; i++)
    {
        std::cout << *(TestTable + i) << " ";
    }
};

Main:

char Signs[] = { '1', '2', '3', '4', '5', '6', 'q', 'w', 'e', 'r', 'f', 'g' };
CTest Test(Signs);

int _tmain(int argc, _TCHAR* argv[])
{
    srand(time(NULL));
    while (Test.AskToPlay())
    {
        if (Test.AskForNewCombination()) Test.NewCombination();
        Test.Play();
    }

    Test.~CTest();
    return 0;
}

Likely, you're using the copy constructor without realizing it. You'll need to provide an example where you use this class that causes the fault. If an object is noncopyable, you should implement that in code as well. [Here's a mechanism to do that](http://stackoverflow.com/questions/2173746/how-do-i-make-this-c-object-non-copyable). — Bill Lynch, Jul 22 '14 at 21:44
This isn't the issue but: Is there a reason you use `*(SignTable + i)` instead of `SignTable[i]`? — wolfPack88, Jul 22 '14 at 21:45
just create a blank private copy constructor and see if you get any errors compiling your code. As sharth said you probably are using the copy constructor without realizing it. — triple_r, Jul 22 '14 at 21:47
@sharth: Those I assumed were because he didn't know about the existence of those functions, which is understandable for a beginner. On the other hand, I find it hard to believe someone can know `*(SignTable + i)` while not knowing `SignTable[i]`. — wolfPack88, Jul 22 '14 at 21:51
@wolfPack88: Yeah. My (now deleted) comment ended up feeling far more snarky than I intended. — Bill Lynch, Jul 22 '14 at 21:59
Since `TestTable` is not a null-terminated string, another possibility might be that it is being treated as one in some cases. — M.M, Jul 22 '14 at 22:11
@wolfPack88 I used to do that.. `*(ptr + index)` way before I learned that it was equal to `ptr[index]`. It was somehow easier to understand.. I don't know where I had learned that from but it was first nature. I remember learning it on a site when learning what a string (`const char*`) was.. — Brandon, Jul 23 '14 at 01:35
It is small private project and I have used ` *(SignTable + i)` instead of ` SignTable[i] ` just for fun :-). @sharth I have studied my code again and again and I really do not realize where I am using the copy constructor without knowing about it so I provide the whole program's code it is quite short. — Al Bundy, Jul 23 '14 at 08:02
What on earth compelled you to call the destructor explicitly? That's automatically done for globals upon program termination. So you have a double deletion going on because of that. Get rid of the call to the destructor. And if you're going to ignore the rule of three four a class that needs it, then declare a private copy constructor and assignment operator and leave them unimplemented. It'll help catch any accidental calls to them. — Praetorian, Jul 23 '14 at 08:24

score 2 · Answer 1 · answered Jul 23 '14 at 08:22

2

You're calling the destructor explicitly at the end of main. It shouldn't be done, the destructor is called automatically at the program end (i.e. when your global Test object goes out of scope). Your explicitly called destructor frees the allocated memory and upon program termination the destructor is called again, attempting to free the memory again, causing the observed failure.

answered Jul 23 '14 at 08:22

SomeWittyUsername

18,025
3
42
85

Thanks a lot, I am just staring with classes and I wrongly thought it is programer's duty to call destructors and the end of the program. – Al Bundy Jul 23 '14 at 08:27
@user3023499 It is programmer's duty in case of **dynamic** allocation of your object (by invoking operator delete), which is not the case here. You have an **automatic** object, which means it is destroyed when going out of scope (including calling the destructor). – SomeWittyUsername Jul 23 '14 at 08:31

Delete[] within destructor, allocating within constructor

1 Answers1