Adding destructor definition creates running exception

Question

I have a problem with a destructor of struct Heap. Even just adding one, and not using it, creates a runtime exception (memory access). It's second day I try to do it and tomorrow is a deadline.

struct Heap
{
       int n;
       int* tab;
       int* numerWKopcu;

       Heap () { n=0; }
       Heap (int size)  { this->tab = new int[liczbaDomow]; n=0; this->numerWKopcu = new int[2000100];}
       int  max()   { return tab[1]; }
       bool empty() { return n==0; }

       bool insert(int x)
       {
            n++;
            tab[n]=x;
            this->numerWKopcu[x] = n;//ZMIANA
            upHeap(n);
            return true;
       }         

       bool delMin()
       {
            if (n<1) return false;
            this->numerWKopcu[tab[n]] = 1; //ZMIANA
            tab[1]=tab[n]; n--;
            downHeap(1);
            return true;
       }

    void upHeap(int x){ 
        int p;
        int mem = tab[x];
        while (x>1)
        {
            p=x/2;
            if (color[mem]>color[tab[p]]) break;
            this->numerWKopcu[tab[p]] = x; //ZMIANA
            tab[x]=tab[p];
            x=p;
        }
        this->numerWKopcu[mem] = x;//ZMIANA
        tab[x]=mem;
    }

    void downHeap (int x)
    {
        int s=2*x;
        int mem=tab[x];
        while(s<=n)
        {
            if (s+1<=n && color[tab[s]]>color[tab[s+1]])
                s++;
            if (color[mem]>color[tab[s]])
            {
                this->numerWKopcu[tab[s]] = x; //ZMIANA
                tab[x]=tab[s];
                x=s;
                s=2*x;
            }
            else break;
        }
        this->numerWKopcu[mem] = x;//ZMIANA
        tab[x]=mem;
    }

    void write ()
    {
        for (int i=1;i<=n;i++) printf ("%d) %d\n", i, tab[i]);
        printf ("\n");
    }       

    void build()
    {
        int s = n;
        for (s=n/2; s>=1; s--) downHeap(s);
    }
    / ~Heap() {
          delete []this->numerWKopcu;
          delete []this-> tab; 
            }; 
};

This code is incomplete (`liczbaDomow` is undefined, for example) and uses non-english variable names. Both together make it very hard for people to help you — thiton, Dec 30 '11 at 14:12
You have Owned RAW pointers and are not following the "Rule of Three" — Martin York, Dec 30 '11 at 15:04
Aside: Everyone who posts to Stack Overflow wants an answer and most want it sooner rather than later, so comments like *"tomorrow is a deadline"* won't win you much sympathy and might be seen as demanding. Probably better to leave them out. — dmckee --- ex-moderator kitten, Dec 30 '11 at 16:08

score 2 · Accepted Answer · edited May 23 '17 at 10:08

2

The code is a bit hard to read, but I see two problems:

You aren't initialising the pointers to null in the default constructor, so destroying a default-constructed object gives undefined behaviour;
You don't define or remove the copy constructor and copy assignment operator (as you should always do if you define a destructor, per the Rule of Three), so destroying a copied object gives undefined behaviour.

It's also possible that you're accessing memory outside the array bounds; a memory debugging tool such as valgrind can help you determine whether that's happening.

The simplest solution is to replace your manually-managed arrays with std::vector; then you won't need to worry about writing your own destructors or copy semantics. You can also use at() rather than [] (at least in a debug variant) to give range-checked access.

edited May 23 '17 at 10:08

Community

1
1

answered Dec 30 '11 at 14:28

Mike Seymour

249,747
28
448
644

It's for ASD-Algorythms and Data Structures I am not allowed to use anything beside #include #include I am not out of the bounds. I am not even intializing the destructor it is just defined. – Yoda Dec 30 '11 at 15:37
@RobertKilar: You might get some more help if you post a short, compilable program that reproduces the problem. As it is, it's hard to do more than point out the obvious problems. – Mike Seymour Dec 30 '11 at 16:02
Ok. Thanks guys I just decided to do not use the delete cause it costs too much. I just initialized int[2000100] cause that's all I need and program got boost from 20 sec to about 0.3 sec. The whole code has 394 lines so it's not readable. Thank You all for the help! – Yoda Dec 30 '11 at 16:11
But I have another question. How much delete []tab costs? – Yoda Dec 30 '11 at 16:19
@RobertKilar: It depends on many things, but typically something like a microsecond on a modern PC. The cost of *not* deleting things is that you'll eventually run out of memory. – Mike Seymour Dec 30 '11 at 16:34

score 1 · Answer 2 · answered Dec 30 '11 at 14:14

1

You are not initializing pointers in default constructor. If you try to destroy default constructed Heap it will try to delete random memory areas in destructor and will definitely break.

answered Dec 30 '11 at 14:14

Tomek

4,554
1
19
19

It is never used. I changed it into: Heap () { n=0; this->tab = new int[liczbaDomow];this->numerWKopcu = new int[2000100]; } but the error stays the same at tab[n] = x in INSERT function. x and n are unknown to the computer, – Yoda Dec 30 '11 at 14:22

Adding destructor definition creates running exception

2 Answers2