How to safely implement an observer pattern?

Question

I implement a mechanism similar to the observer design pattern for the multi-threaded Tetris game. There is a Game class that contains a collection of EventHandler objects. If a class wants to register as a listener for a Game object, it must inherit the Game :: EventHandler class. When state changes occur on the EventHandler interface of each listener, the corresponding method is called. This is what the code looks like:

class Game
{
public:
    class EventHandler
    {
    public:
        EventHandler();

        virtual ~EventHandler();

        virtual void onGameStateChanged(Game * inGame) = 0;

        virtual void onLinesCleared(Game * inGame, int inLineCount) = 0;

    private:
        EventHandler(const EventHandler&);
        EventHandler& operator=(const EventHandler&);
    };

    static void RegisterEventHandler(ThreadSafe<Game> inGame, EventHandler * inEventHandler);

    static void UnregisterEventHandler(ThreadSafe<Game> inGame, EventHandler * inEventHandler);

    typedef std::set<EventHandler*> EventHandlers;
    EventHandlers mEventHandlers;

private:    
    typedef std::set<Game*> Instances;
    static Instances sInstances;
};


void Game::RegisterEventHandler(ThreadSafe<Game> inGame, EventHandler * inEventHandler)
{
    ScopedReaderAndWriter<Game> rwgame(inGame);
    Game * game(rwgame.get());
    if (sInstances.find(game) == sInstances.end())
    {
        LogWarning("Game::RegisterEventHandler: This game object does not exist!");
        return;
    }

    game->mEventHandlers.insert(inEventHandler);
}


void Game::UnregisterEventHandler(ThreadSafe<Game> inGame, EventHandler * inEventHandler)
{
    ScopedReaderAndWriter<Game> rwgame(inGame);
    Game * game(rwgame.get());
    if (sInstances.find(game) == sInstances.end())
    {
        LogWarning("Game::UnregisterEventHandler: The game object no longer exists!");
        return;
    }

    game->mEventHandlers.erase(inEventHandler);
}

There are two problems that I often encounter with a similar pattern:

• The listener wants to unregister at the already deleted object, which will lead to a failure.
• An event is fired by a listener that no longer exists. This is most often found in multi-threaded code. Here's a typical scenario:
  • . , .
  • boost:: function PostMessage .
  • , Game . .

- , . UnregisterEventHandler, . , .

- , /? , ?

PS: , , : Game.h, Game.cpp, SimpleGame.h, SimpleGame.cpp, MainWindow.cpp.

Answer 1

• , delete new . . , . , .

• , . , , . shared_ptr , , .

Answer 2

++ Observer , . -, " ", " " .., . , ... , . . , , , - - (API-), , .

, ++, (, boost, Qt ..), ( , ) .

, :

• ( , ), . , .
• . Notify (...), , , Observer ( ) .
• , Detach (...), Notify (...), .
• Notify (...) Observer, Subject. , . , Notify (...). .
• (...) Observer (A B, C, D D.Notify(...) Notify (...) E ..).

:

/* 
 The Notifier is a singleton implementation of the Subject/Observer design
 pattern.  Any class/instance which wishes to participate as an observer
 of an event can derive from the Notified base class and register itself
 with the Notiifer for enumerated events.

 Notifier derived classes MUST implement the notify function, which has 
 a prototype of:

 void Notify(const NOTIFIED_EVENT_TYPE_T& event)

 This is a data object passed from the Notifier class.  The structure 
 passed has a void* in it.  There is no illusion of type safety here 
 and it is the responsibility of the user to ensure it is cast properly.
 In most cases, it will be "NULL".

 Classes derived from Notified do not need to deregister (though it may 
 be a good idea to do so) as the base class destrctor will attempt to
 remove itself from the Notifier system automatically.

 The event type is an enumeration and not a string as it is in many 
 "generic" notification systems.  In practical use, this is for a closed
 application where the messages will be known at compile time.  This allows
 us to increase the speed of the delivery by NOT having a 
 dictionary keyed lookup mechanism.  Some loss of generality is implied 
 by this.

 This class/system is NOT thread safe, but could be made so with some
 mutex wrappers.  It is safe to call Attach/Detach as a consequence 
 of calling Notify(...).  

 */


class Notified;

class Notifier : public SingletonDynamic<Notifier>
{
public:
   typedef enum
   {
      NE_MIN = 0,
      NE_DEBUG_BUTTON_PRESSED = NE_MIN,
      NE_DEBUG_LINE_DRAW_ADD_LINE_PIXELS,
      NE_DEBUG_TOGGLE_VISIBILITY,
      NE_DEBUG_MESSAGE,
      NE_RESET_DRAW_CYCLE,
      NE_VIEWPORT_CHANGED,
      NE_MAX,
   } NOTIFIED_EVENT_TYPE_T;

private:
   typedef vector<NOTIFIED_EVENT_TYPE_T> NOTIFIED_EVENT_TYPE_VECTOR_T;

   typedef map<Notified*,NOTIFIED_EVENT_TYPE_VECTOR_T> NOTIFIED_MAP_T;
   typedef map<Notified*,NOTIFIED_EVENT_TYPE_VECTOR_T>::iterator NOTIFIED_MAP_ITER_T;

   typedef vector<Notified*> NOTIFIED_VECTOR_T;
   typedef vector<NOTIFIED_VECTOR_T> NOTIFIED_VECTOR_VECTOR_T;

   NOTIFIED_MAP_T _notifiedMap;
   NOTIFIED_VECTOR_VECTOR_T _notifiedVector;
   NOTIFIED_MAP_ITER_T _mapIter;

   // This vector keeps a temporary list of observers that have completely
   // detached since the current "Notify(...)" operation began.  This is
   // to handle the problem where a Notified instance has called Detach(...)
   // because of a Notify(...) call.  The removed instance could be a dead
   // pointer, so don't try to talk to it.
   vector<Notified*> _detached;
   int32 _notifyDepth;

   void RemoveEvent(NOTIFIED_EVENT_TYPE_VECTOR_T& orgEventTypes, NOTIFIED_EVENT_TYPE_T eventType);
   void RemoveNotified(NOTIFIED_VECTOR_T& orgNotified, Notified* observer);

public:

   virtual void Reset();
   virtual bool Init() { Reset(); return true; }
   virtual void Shutdown() { Reset(); }

   void Attach(Notified* observer, NOTIFIED_EVENT_TYPE_T eventType);
   // Detach for a specific event
   void Detach(Notified* observer, NOTIFIED_EVENT_TYPE_T eventType);
   // Detach for ALL events
   void Detach(Notified* observer);

   /* The design of this interface is very specific.  I could 
    * create a class to hold all the event data and then the
    * method would just have take that object.  But then I would
    * have to search for every place in the code that created an
    * object to be used and make sure it updated the passed in
    * object when a member is added to it.  This way, a break
    * occurs at compile time that must be addressed.
    */
   void Notify(NOTIFIED_EVENT_TYPE_T, const void* eventData = NULL);

   /* Used for CPPUnit.  Could create a Mock...maybe...but this seems
    * like it will get the job done with minimal fuss.  For now.
    */
   // Return all events that this object is registered for.
   vector<NOTIFIED_EVENT_TYPE_T> GetEvents(Notified* observer);
   // Return all objects registered for this event.
   vector<Notified*> GetNotified(NOTIFIED_EVENT_TYPE_T event);
};

/* This is the base class for anything that can receive notifications.
 */
class Notified
{
public:
   virtual void Notify(Notifier::NOTIFIED_EVENT_TYPE_T eventType, const void* eventData) = 0;
   virtual ~Notified();

};

typedef Notifier::NOTIFIED_EVENT_TYPE_T NOTIFIED_EVENT_TYPE_T;

. Notified , Notify (...) . void * , , :

virtual void Notify(Notifier::NOTIFIED_EVENT_TYPE_T eventType, int value); 
virtual void Notify(Notifier::NOTIFIED_EVENT_TYPE_T eventType, const string& str);

Notify (...) Notifier. " ", . .

, , . . , .