Problems implementing the "Observer" pattern

Question

I have met an interesting problem while implementing the Observer pattern with C++ and STL. Consider this classic example:

class Observer {
public:
   virtual void notify() = 0;
};

class Subject {
public:
   void addObserver( Observer* );
   void remObserver( Observer* );
private:
   void notifyAll();
};

void Subject::notifyAll() {
   for (all registered observers) { observer->notify(); }
}

This example can be found in every book on design patterns. Unfortunately, real-life systems are more complex, so here is the first problem: some observers decide to add other observers to the Subject on being notified. This invalidates the "for" loop and all the iterators, that I use. The solution is rather easy - I make a snapshot of the registered observers list and iterate over the snapshot. Adding new observers does not invalidate the snapshot, so everything seems ok. But here comes another problem: observers decide to destroy themselves on being notified. Even worse, one single observer can decide to destroy all other observers (they are controlled from the scripts), and that invalidates both the queue and a snapshot. I find myself iterating over de-allocated pointers.

My question is how should I handle the situations, when observers kill each other? Are there any ready-to-use patterns? I always thought that "Observer" is the easiest design pattern in the world, but now it seems it is not that easy to implement it correctly...

Thank you, everyone for your interest. Let us have a decisions summary:

[1] "Don't do it" Sorry, but it is a must. Observers are controlled from the scripts and are garbage-collected. I cannot control the garbage collection to prevent their de-allocation;

[2] "Use boost::signal" The most promising decision, but I cannot introduce boost on the project, such decisions must be made by the project leader only (we are writing under Playstation);

[3] "Use shared__ptr" That will prevent observers from de-allocation. Some sub-systems may rely on memory pool cleanup, so I don't think I can use shared_ptr.

[4] "Postpone observer deallocation" Queue observers for removal while notifying, then use the second cycle to delete them. Unfortunately, I cannot prevent the deallocation, so I use a trick of wrapping observer with some kind of "adaptor", keeping actually the list of "adaptors". On destructor, observers unassign from their adaptors, then I take my second cycle to destroy empty adaptors.

p.s. is it ok, that I edit my question to summarize all the post? I am noob on StackOverflow...

Answer 1

How about using a linked list in your for loop ?

Answer 2

If your program is multi-threaded you may need to use some locking here.

Anyway, from your description it seems that the issue is not concurrency (multi-thrading) but rather mutations induced by the Observer::notify() call. If this is the case then you can solve the problem by using a vector and traversing it via an index rather than an iterator.

for(int i = 0; i < observers.size(); ++i)
  observers[i]->notify();

Answer 3

Personally, I use boost::signals to implement my observers; I'll have to check, but I believe it handles the above scenarios (edited: found it, see "When can disconnections occur"). It simplifies your implementation, and it doesn't rely on creating custom class:

class Subject {
public:
   boost::signals::connection addObserver( const boost::function<void ()>& func )
   { return sig.connect(func); }

private:
   boost::signal<void ()> sig;

   void notifyAll() { sig(); }
};

void some_func() { /* impl */ }

int main() {
   Subject foo;
   boost::signals::connection c = foo.addObserver(boost::bind(&some_func));

   c.disconnect(); // remove yourself.
}

Answer 4

A man goes to the doctor and says, "Doc when I raise my arm like this it hurts real bad!" The doctor says, "Don't do that."

The simplest solution is to work with your team and tell them to not do that. If observers "really need" to kill themselves, or all observers, then schedule the action for when the notification is over. Or, better yet, change the remObserver function to know if there's a notify process happening and just queue up the removals for when everything is done.

Answer 5

The problem is that of ownership. You could use smart pointers, for instance the boost::shared_ptr and boost::weak_ptr classes, to extend the lifetime of your observers past the point of "de-allocation".

Answer 6

There are several solutions for this problem:

1. Use boost::signal it allows automatic connection removal when the object destroyed. But you should be very careful with thread safety
2. Use boost::weak_ptr or tr1::weak_ptr for managment of observers, and boost::shared_ptr or tr1::shared_ptr for observers them self -- reference counting would help you for invalidating objects, weak_ptr would let you know if object exists.

3. If you are running over some event loop, make sure, that each observer does not destroy itself, add itself or any other in the same call. Just postpone the job, meaning

   SomeObserver::notify()
   {
      main_loop.post(boost::bind(&SomeObserver::someMember,this));
   }
   

Answer 7

Very interesting issue.

Try this:

1. Change remObserver to null out the entry, rather than just removing it (and invalidating the list iterators).

2. Change your notifyAll loop to be:

   for (all registered observers) { if (observer) observer->notify(); }

3. Add another loop at the end of notifyAll to remove all null entries from your observer list

Answer 8

How about having a member iterator called current (initialised to be the end iterator). Then

void remObserver(Observer* obs)
{
    list<Observer*>::iterator i = observers.find(obs);
    if (i == current) { ++current; }
    observers.erase(i);
}

void notifyAll()
{
    current = observers.begin();
    while (current != observers.end())
    {
        // it's important that current is incremented before notify is called
        Observer* obs = *current++;
        obs->notify(); 
    }
}

Answer 9

Define and use a heavy duty iterator over the container of notifiers that is resilient to deletion (eg, nulling out, as previously mentioned) and can handle addition (eg appending)

On the other hand if you want to enforce keeping the container const during notification, declare notifyAll and the container being iterated on as const.

Answer 10

Here's a variation on the idea T.E.D. already presented.

As long as remObserver can null an entry instead of immediately removing it, then you could implement notifyAll as:

void Subject::notifyAll()
{
    list<Observer*>::iterator i = m_Observers.begin();
    while(i != m_Observers.end())
    {
        Observer* observer = *i;
        if(observer)
        {
            observer->notify();
            ++i;
        }
        else
        {
            i = m_Observers.erase(i);
        }
    }
}

This avoids the need for a second clean-up loop. However it does mean that if some particular notify() call triggers the removal of itself or an observer located earlier in the list, then the actual removal of the list element will be deferred until the next notifyAll(). But as long as any functions which operate over the list take proper care to check for null entries when appropriate, then this shouldn't be a problem.

Answer 11

This is a bit slower since you are copying the collection, but I think it's simpler too.

class Subject {
public:
   void addObserver(Observer*);
   void remObserver(Observer*);
private:
   void notifyAll();
   std::set<Observer*> observers;
};

void Subject::addObserver(Observer* o) {
  observers.insert(o);
}

void Subject::remObserver(Observer* o) {
  observers.erase(o);
}

void Subject::notifyAll() {
  std::set<Observer*> copy(observers);
  std::set<Observer*>::iterator it = copy.begin();
  while (it != copy.end()) {
    if (observers.find(*it) != observers.end())
      (*it)->notify();
    ++it;
  }
}

Answer 12

Ladies and Gentlemen please welcome .Net 4.0 with its newly introduced interfaces IObserver and IObservable, which will help in building loosely coupling system between the data provider and the observers. Whilst IObservable provides all the functionality for the publisher, IObserver does the same for subscriber. You will find that publisher and subscriber are also known as provider and observer. Whatever name is used for, keep in mind what each item is supposed to do, otherwise you will feel confused.

http://www.jaftalks.com/Home/Show/Observer-Design-Pattern-DotNet4