Ideas for memory management, std::map, boost::shared_ptr

Question

This question is long so please bear with me.

I'm attempting to solve a dilemma I'm having with memory management, shared pointers, and maps. I just wanted to get some feedback about my architecture, maybe some of ya'll have done it better in the past.

The following examples will be done is pseudo code.

I have a listener class:

class MyListener {
   friend class Command;
   public:
      MyListener() {}
      virtual ~MyListener() {}
      void handleUpdate() {
           std::cout << "Update Handled" << std::endl;
      }
};

Which is to be evoked everytime an object update is called. I'm using a middleware called OpenDDS for interprocess communication framework.

I have a Command class, which inherits a DDS object, and leverages the on_data_received(). When on_data_received() is evoked, I want to call the handleUpdate() method from the class above.

class Command {
   public:
      /*standard constructor destructor here*/
      void on_data_received() {
          m_listener->handleUpdate();
      }
      void write();
   private:
      MyListener *m_listener;
 };

Herein lies the problem. The class that manages this is a Singleton, and uses two methods publish and subscribe to either publish a DDS message or subscribe to one. The subscribe method takes a key value and a raw pointer.

The singleton manages a

std::map<std::string name, Command>

In which a Command class contains the MyListener class.

Here is a snippet of pseudo code that breaks it:

class TaterTotListener : public MyListener {
     void handleCommand() {
         std::cout << "Tater tot found" << std::endl;
     }
};

int main() {
    // make a new smart pointer to the listener
    boost::shared_ptr<TaterTotListener> ttl(new TaterTotListener);
    // tell the singleton we want to publish an object called "TaterTot"
    CommandManager::instance()->publish("TaterTot");
    // tell the singleton we want to subscribe to an object called tater tot
    CommandManager::isntance()->subscribe("TaterTot", ttl.get());

    // processing goes here
    // deallocation

}

Upon deallocation, boost is removing it's ownership of the shared pointer. CommandManager attempts to "clean up" by removing all objects named "TaterTot", but since boost::shared_ptr has already cleaned itself up, a double free memory corruption is thrown. CommandManager singleton is always cleaned up last, so declaring a raw pointer and passing to the subscribe method is going to result in the same behaviour.

Any ideas out there? Did I miss something obvious and intuitive? Am I misunderstanding the usage of shared pointers in this instance?

Any help is greatly appreciated. I'll buy ya'll a beer.

Answer 1

Your design mixes two well-known design patterns, Observer and Command.

Observer defines a one-to-many dependency between objects so that when one object changes state, all its dependent clients are notified and updated automatically.

Command encapsulates a request as an object, thereby letting you parameterize clients with different requests, queue or log requests, and support undoable operations.

I would recommend studying those patterns (see the links above) and refactor your design to separate the encapsulation of requests from the observation of those requests.

Answer 2

First of all, I would recommend to follow rhalbersma's advice in another answer. Also, I would double check if you need a singleton. I don't see the strong need of it from what you described.

Technically it's a problem of ownership. The info you provided requires me to guess couple of things. I suppose MyListener instance should be shared between different things, a Command object and something else (since you use shared_ptr). So you need really share this ownership by:

class Command {
   private:
      boost::shared_ptr<MyListener> m_listener;
};

and

CommandManager::isntance()->subscribe("TaterTot", ttl);

So MyListener will be deallocated by the last owner.

Answer 3

Thanks to contributors, I've managed to create a minimal, compilable example of exactly what I wanted using Observer Design Pattern as suggested by @rhalbersma.

#include <cstdlib>
#include <iostream>
#include <map>
#include <boost/shared_ptr.hpp>
#include <vector>

class Listener {
public:
    virtual void handleUpdate() = 0;
    std::string *string;
};

class Command {
    std::vector<boost::shared_ptr<Listener> > listener;

public:

    ~Command() {
    }

    void setListener(boost::shared_ptr<Listener> l) {
        listener.push_back(l);
    }

    void handleCommand() {
        for (int i = 0; i < listener.size(); i++) {
            //std::cout << "Handle command " << i << std::endl;
            listener.at(i)->string = string;
            listener.at(i)->handleUpdate();
        }
    }

    void write(std::string value) {
        std::cout << "Write 1" << std::endl;
        string = &value;
        handleCommand();
    }

private:
    std::string *string;
};

class MyListener : public Listener {
public:

    MyListener(int num) : num(num) {
    };

    ~MyListener() {
    }

    void handleUpdate() {
        std::cout << "Listener " << num << " with string " << *string << std::endl;
    }

private:

    int num;
};

class CommandManager {
public:

    void publish(std::string name, std::string value) {
        std::map<std::string, boost::shared_ptr<Command> >::iterator it;
        it = commandMap.find(name);
        if (it == commandMap.end()) {
            // add a new one
            boost::shared_ptr<Command> command(new Command());
            commandMap[name] = command;
        }

        it = commandMap.find(name);
        it->second->write(value);
    }

    void subscribe(std::string name, boost::shared_ptr<Listener> l) {
        std::map<std::string, boost::shared_ptr<Command> >::iterator it;
        it = commandMap.find(name);
        if (it == commandMap.end()) {
            boost::shared_ptr<Command> command(new Command());
            command->setListener(l);
            commandMap[name] = command;
        } else {

            it->second->setListener(l);
        }
    }        

private:

    std::map<std::string, boost::shared_ptr<Command> > commandMap;
};

int main(int argc, char** argv) {

    boost::shared_ptr<MyListener> myListener0(new MyListener(0));
    boost::shared_ptr<MyListener> myListener1(new MyListener(1));
    boost::shared_ptr<MyListener> myListener2(new MyListener(2));

    CommandManager commandManager;

    commandManager.subscribe("Tyler", myListener0);
    commandManager.subscribe("Tyler", myListener1);
    commandManager.subscribe("Tyler", myListener2);

    commandManager.publish("Tyler", " is cool");

    return 0;
}

I'm including the full source for Google users who may be stuck on the same problem. Once again, Thank you StackOverflow.