[RESOLVED] STL container static to class and avoiding raw pointers

[grillon_bleu]

Hi,

I define a class which has a std::map static data member that i use it to store a limited amount of instances (the constructor is private). A static class method TheClass::instance(map_index) returns a reference to the instance corresponding to the supplied map_index. The map is filled at runtime, when the static method TheClass::instance is used for the first time. (The instances contain data loaded from files thus i use this technique so that the data is loaded only once).

My question is this: since i fill the map dynamically at runtime (using new), at what point do i actually free it (using delete)? This means i'd have a map of raw pointers. Would i be able to instead have a map of instances or smart pointers? I can't see where the memory of the instances would be (can't be on the stack, and if it's on the heap, at some point i need to free it), and the smart pointer itself would have the same problem.

Here is the definition of the method which fills the map (simplified for clarity):

void Sector::getDefinedSectors() {
  DIR *pdir;
  struct dirent *pent;

  pdir=opendir(dataPath);

  while((pent = readdir(pdir))) {
    std::string filename = dataPath + std::string(pent->d_name);
    if(fileIsModel(filename)) {
      Sector* s = new Sector(filename);
      const Secteur::Openings& openings = s.openings();
      definedSectors.insert(std::map<openings, *Sector>::value_type(openings, s));
    }
  }

}

In fact, I don't even see when i'd actually iterate through the map to delete the instances! I'd have to have a static cleanup method to manually free the map... ugh. Is there a more robust way to do this?

[Lindley]

I suppose you could replace the static map with a "SectorMap" class that owns a map of Sectors. The Sectors could then be free'd in the SectorMap's destructor.

Of course, this is only really useful if you only expect these things to live for part of the duration of your program, and then you want the memory back. If you expect them to stick around until program termination once they're created, there's really no harm in leaving them allocated since you've put a limit on how many of them there can be anyway.

Memory leaks are only a problem if they get worse over time, really.

[HighCommander4]

Hi,

I define a class which has a std::map static data member that i use it to store a limited amount of instances (the constructor is private). A static class method TheClass::instance(map_index) returns a reference to the instance corresponding to the supplied map_index. The map is filled at runtime, when the static method TheClass::instance is used for the first time. (The instances contain data loaded from files thus i use this technique so that the data is loaded only once).

My question is this: since i fill the map dynamically at runtime (using new), at what point do i actually free it (using delete)? This means i'd have a map of raw pointers. Would i be able to instead have a map of instances or smart pointers? I can't see where the memory of the instances would be (can't be on the stack, and if it's on the heap, at some point i need to free it), and the smart pointer itself would have the same problem.

Here is the definition of the method which fills the map (simplified for clarity):

void Sector::getDefinedSectors() {
  DIR *pdir;
  struct dirent *pent;

  pdir=opendir(dataPath);

  while((pent = readdir(pdir))) {
    std::string filename = dataPath + std::string(pent->d_name);
    if(fileIsModel(filename)) {
      Sector* s = new Sector(filename);
      const Secteur::Openings& openings = s.openings();
      definedSectors.insert(std::map<openings, *Sector>::value_type(openings, s));
    }
  }

}

In fact, I don't even see when i'd actually iterate through the map to delete the instances! I'd have to have a static cleanup method to manually free the map... ugh. Is there a more robust way to do this?

You can use the smart pointer in this way:

void Sector::getDefinedSectors() {
  DIR *pdir;
  struct dirent *pent;

  pdir=opendir(dataPath);

  while((pent = readdir(pdir))) {
    std::string filename = dataPath + std::string(pent->d_name);
    if(fileIsModel(filename)) {
      boost::shared_ptr<Sector> s(new Sector(filename));  // or other smart pointer type
      const Secteur::Openings& openings = s.openings();
      definedSectors.insert(std::make_pair(openings, s));
    }
  }

}

definedSectors would then have to be of type

std::map<Opening, shared_ptr<Sector> >

Then you don't need to worry about deleting anything - the smart pointer will take care of that for you.

(I also replaced the map<...>::value_type in the insertion with std::make_pair - it makes the code look much cleaner.)

[grillon_bleu]

Lindley thanks for your reply. Wrapping the std::map inside a SectorMap class would allow me to get an handle on the destructor of the static data member, of course! I think this is what i will use. The map would live for the whole duration of the program execution, and i wouldnt have expected this small memory leak to really cause trouble, but i usually use higher level languages which takes care of this stuff, so i want to make sure i do this memory management thing right before it gets out of hand.

HighCommander4, in your code example, isn't your boost::shared_ptr<Sector> instance allocated on the stack and then passed by reference (i think that's how make_pair takes its arguments)? Once the scope finishes, isn't the share_ptr instance facing destruction? Thank you for your clarification.

[MrViggy]

Well, technically, if the pointers are in use until the program terminates, then there really isn't a memory leak (in the traditional sense). Once your program terminates, the OS *should* return all it's memory to the system.

Viggy

[HighCommander4]

HighCommander4, in your code example, isn't your boost::shared_ptr<Sector> instance allocated on the stack and then passed by reference (i think that's how make_pair takes its arguments)? Once the scope finishes, isn't the share_ptr instance facing destruction? Thank you for your clarification.

boost::shared_ptr is a reference-counted smart pointer class. You allocate the shared_ptr object on the stack, and the shared_ptr object allocates the Sector object on the heap. A reference count of 1 is associated with the Sector object, because there is one reference (shared_ptr) that points to that object. When you pass the shared_ptr into the map, a copy of the shared_ptr is made. At this point, two shared_ptr objects (the original one you created on the stack, and the copy managed by the map) refer to the same Sector object, so the reference count is now incremented to 2. When the shared_ptr on the stack goes out of scope, its destructor gets called, but the destructor does not delete the Sector object, because the reference count is 2, and the Sector object is only destructed when the destructor of the last reference to it is destructed (think garbage collection - this is what shared_ptr emulates). The destructor of the shared_ptr object on the stack simply decrements the reference count back to 1. Then, when the map itself goes out of scope at the end of the program, it calls the destructor of the shared_ptr that it manages, which in turn destroys the Sector object since the reference count is now 1 and the last reference is being destroyed.

[grillon_bleu]

So the STL containers store only copies that they manage themselves, that makes sense. I played around with it and some made up test classes with verbose constructors, copy constructors and destructors and both the keys and values in maps are necessarly copies. So i'll use boost::shared_ptr. Fairly automatic garbage collection is exactly what i'm trying to have. Thanks for the clarifications!