c++ how to free a static reference to an outside object?

Question

First you need to look into this code, here I have two classes -- one is the TreasureBag and the other is the Sprite. I have multiple copies of sprite but there is only one bag (hence it's static). The sprites acquire the bag and collect treasures, store inside it, then the next sprite comes and use it for treasure collection.

Here is my code --

class TreasureBag {
    private:
        int* bag ; int size ; int index ;

    public:
        TreasureBag(int size = 10) {
            this->index = 0 ; this->size = size ;
            this->bag = new int[this->size] ;
        }
        ~TreasureBag() { delete [] this->bag ; }
        void put(int data) { this->bag[this->index++] = data ;}
        friend ostream& operator<< (ostream &os, const TreasureBag &tb);
};

class Sprite 
{
    private:
        static TreasureBag tbag ; int points = 0 ; string name ;

    public:
        Sprite(string name) { this->name = name ; }
        ~Sprite() { ; } // --> not sure what to do here
        void collect(int data) { this->tbag.put(data); }
        void acquireTreasureBag(TreasureBag &tb) { Sprite::tbag = tb ; }
        void releaseTreasureBag() { ; } // ---> not sure what to do here either
        friend ostream& operator<< (ostream& os, const Sprite &s);
};
TreasureBag Sprite::tbag ;

Inside the main, I have two sprites s1 and s2, and one treasure bag. The sprite s1 comes first, grabs it and put stuffs inside, then it releases the bag and s2 acquires it.

int main()
{
    TreasureBag tb(5) ; Sprite s1("s1");
    s1.acquireTreasureBag(tb); // <-- Looks like this line is problematic

    /*s1.collect(1); s1.collect(2);
    cout << s1 << endl ;
    s1.releaseTreasureBag();

    Sprite s2("s2"); s2.acquireTreasureBag(tb);
    s2.collect(3);
    cout << s2 << endl ;*/
    return 0 ;
}

But when I run the code (please see the un-commented lines), I get this error --

*** Error in `./a.out': double free or corruption (fasttop): 0x0000000002495040 ***
Aborted (core dumped)

What is the best way to handle this kind of situation? Is my design principle ok? Moreover, I am not sure what I should do in the sprite destructor ~Sprite() and the releaseTreasureBag() procedure.

please help.

Which line is the error happening on? You don't need to do anything in the destructor, because `tbag` isn't a pointer. It's a copy of the `TreasureBag` that was acquired. — Barmar, Dec 20 '14 at 00:43
This use of `static` makes no sense, especially when you are using it with `this->`. — T.C., Dec 20 '14 at 00:43
Your copy-assignment operator fails to perform a deep copy on the resource, so on the line `Sprite::tbag = tb` you now have two pointers to the same `bag` which is deleted twice by the destructors of the two instances. Please read [What is The Rule of Three?](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — David G, Dec 20 '14 at 00:54
Implementation details aside, there are serious design problems with this code. You seem to want `TreasureBag` to be a singleton, yet you construct two instances of it. You seem to want one sprite to have exclusive ownership of the bag at a time, yet the one of the `TreasureBag` instances is shared across all sprites at all times. — T.C., Dec 20 '14 at 01:10

score 3 · Accepted Answer · answered Dec 20 '14 at 00:57

You don't respect rule of three (or Five). Use std::vector to avoid to manage memory yourself:

class TreasureBag {
private:
    std::vector<int> bag;
    int max_size;

public:
    TreasureBag(int size = 10) : max_size(size) {}
    void put(int data) { if (bag.size() < max_size) bag.push_back(data) ;}
public:
    friend ostream& operator<< (ostream &os, const TreasureBag &tb);
};

score 1 · Answer 2 · answered Dec 20 '14 at 00:42

1

It seems like you might want to use smart pointers for this:

class Sprite {
    static std::shared_ptr<TreasureBag> tbag;

public:
    void acquire(std::shared_ptr<TreasureBag> tb) {
        tbag = tb;
    }

    void release() {
        tbag.reset();
    }
};

And then construct your TreasureBag as a shared_ptr too:

std::shared_ptr<TreasureBag> tb = std::make_shared<TreasureBag>(5);
Sprite s1("s1");
s1.acquire(tb);

answered Dec 20 '14 at 00:42

Barry

286,269
29
621
977

thanks, but how should I call to the member function of `TreasureBag` over the `shared_ptr` ? – ramgorur Dec 20 '14 at 00:58
what I mean what should I do to call `tb.put()` ?? – ramgorur Dec 20 '14 at 01:04
@ramgorur `tb->put()`. – Barry Dec 20 '14 at 01:05

c++ how to free a static reference to an outside object?

2 Answers2