2

I discovered this accidentally today when I was working on my project. Basically, in my project, I have something similar to below for resource handling.

class Resource {
public:
    static Resource instance_;
    ~Resource () {
        std::for_each(res_.begin(), res_.end(), [&] (Stuff *stuff) {
            delete stuff;
        });
    }
private:
    std::set<Stuff*> res_;
};

Running this again valgrind, upon program exit, I'm seeing some really cryptic errors. Something like this:

by 0x40DD42: std::_Rb_tree<unsigned int, std::pair<unsigned int const, std::pair<Vertex*, unsigned int> >, 
std::_Select1st<std::pair<unsigned int const, std::pair<Vertex*, unsigned int> > >, std::less<unsigned int>, 
std::allocator<std::pair<unsigned int const, std::pair<Vertex*, unsigned int> > > >::equal_range(unsigned int const&)
\\ a lot, lot more to come...

Reading through all this, it seems to indicate that the destructor of Resource is freeing already freed memory region.

But my destructor is definitely handling things correctly. To prove this, I moved the deletion code out of the destructor into another member function. So, something like this:

class Resource {
public:
    static Resource instance_;
    ~Resource () { /* does nothing */ }
    void clear () {
        std::for_each(res_.begin(), res_.end(), [&] (Stuff *stuff) {
            delete stuff;
        });
    }
// ... more
};

Then, I simply call clear() on the static instance before the program exits. Now, the error no longer appears in valgrind!

To further prove that this is only to do with when a static instance dies when the program dies, I removed the static instance. Instead of having a static instance_, I just allocated an instance of Resource on stack in main() when my program starts. After this change, the problem also goes away.

Now, I'm wondering why this is happening? Does this have anything to do with the operating system?

My guess is that the OS probably attempts to free everything as the program dies, and my destructor just happens to kick in during the clean-up, not before.

The OS in question here is Linux (Ubuntu 12.10). The compiler is gcc 4.7.2.

Thanks

Jimmy Lu
  • 4,810
  • 7
  • 25
  • 30
  • Not quite a dup, but some interesting information in [this question](http://stackoverflow.com/questions/2204608/does-c-call-destructors-for-global-and-class-static-variables) – Collin Oct 31 '12 at 23:31
  • Are you sure it's not reporting unfreed memory, not that it's being freed twice? What happens if you don't use a lamba, and instead loop over it normally, or use a functor? – Collin Oct 31 '12 at 23:34
  • @Collin, it is reporting that Im freeing the region twice. I tried with a functor passed into for_each, I got the same error. – Jimmy Lu Oct 31 '12 at 23:36
  • How does `Stuff` look like? May those `Stuff` instances be freed just before main() ends? – timrau Nov 01 '12 at 00:28
  • I can't understand the reason of error from your posted data, but why you do not `clear` the `set` in destructor to avoid the error? – BigBoss Nov 01 '12 at 01:07

2 Answers2

2

I'm running Xubuntu 12.10, my gcc version matches yours:

$ gcc --version
gcc (Ubuntu/Linaro 4.7.2-2ubuntu1) 4.7.2

With this test case:

#include <set>
#include <algorithm>

class Thing {};

class Test {
public:
    static Test test;

    std::set<Thing*> things;

    ~Test() {
        std::for_each(things.begin(), things.end(), 
                      [&](Thing* thing){ 
                          delete thing; 
                      });
    } 
};

Test Test::test;

int main() {
    Test::test.things.insert(new Thing());
    Test::test.things.insert(new Thing());

    return 0;
}

And valgrind reports everything is fine, 4 allocations, 4 frees. What happens if you try the same test case?

Collin
  • 11,977
  • 2
  • 46
  • 60
0

You forgot to implement a [deep] copy constructor and associated assignment operator.

What happens when you create a copy of an instance of Resource? The first copy eventually gets destroyed, and then the second copy eventually gets destroyed... and each copy's destruction leads to the deleteion of the same pointers. Not good.

In short, you forgot to implement the rule of three. Look this up in your C++ book's glossary, since it'll certainly be there.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
  • I know about the rule of three. In my actual project, I did disable them, I never used more than one instance of Resource in my project (at least until now). Now that I got rid of the whole static instance thing, I may consider implementing deep copy in the future. – Jimmy Lu Nov 01 '12 at 02:43