Observer设计模式的陷阱，兼谈C++语言在模式面前的悲哀

[skyMountain]

前几天，刚写的一个软件崩溃了。跟踪发现是下面函数的问题：
void CSubject::OnMsg(CSMSG *pMsg)
{
for(list<IMsgListener*>::iterator it = m_lstMsgListener.begin();
it != m_lstMsgListener.end(); it ++)
{
ASSERT( NULL != (*it) );
(*it) ->Notify(pMsg);
}
}

有经验的人一看就知道这是使用的是GOF的“观察者模式”。m_lstListener是观察者集合，用户可以通过Attach或者Detach函数在 m_lstListener中增加或删除观察者。当观察者期待的某个时间产生时，通过上面这一段代码，每个观察者都可以收到相应的消息（通过Notify 接口）。
可是，相信熟悉STL的人看到这一段代码，第一时间都会想：这会不会导致迭代子失效？
这段代码里，Notify接口是个虚函数。也就是说，Notify函数的具体实现，是由使用者自行决定的。上面这段代码，本身并不会引起迭代子失效，但是如果用户在Notify函数里不慎修改了m_lstListener，那么这段代码就会崩溃。
也就是说，观察者如果在Notify函数中调用Attach或者Detach函数，迭代子就会失效，系统就会崩溃！
接触设计模式已经有两三年了，观察者模式是我最常用的模式之一。没想到这个简单的设计，竟然会有这么严重的问题。
奇怪的是，为什么那么多有关设计模式的书籍，对这个那么容易出错的问题只字不提呢？网上那么多设计模式的实现例子，都采用这个有严重错误的设计呢？甚至在GOF那本名著中，采用的也是这种类似的设计（只不过是用java语言而已）。谬种流传，害人不浅。

可能这也是C++语言的悲哀吧。jdk库中，已经有现成的observer模式支持，用户根本不用写这些代码。C++程序员却只能一行行自己编写，谁叫C++没有一个类似与JDK的基础库呢？
很多设计模式，C++语言实现起来都非常困难。就连最简单的工厂方法，C++也难以实现。用一个函数专门负责对象的生成，那么这个对象由谁负责释放，就成了问题。工厂模式直接违反了C++语言的基本编码规范：谁申请的内存，谁负责释放。JAVA中有垃圾回收机制，生成的对象不用关心如何释放，所以工厂模式使用起来得心应手。但C++程序员呢？高级一点的还可能会使用智能指针来解决这个问题，但那些连boost库都没听过过的程序员呢？他们就连工厂模式都用不好了。

那么这个问题如何解决呢？并不好解决。在单线程环境下，我建议用“观察者队列缓存”的方式：
一、增加一个bool型的迭代标志，标志观察者队列是否处于迭代之中。即在OnMsg函数进入的时候，将其设为true，出去的时候设为false。
二、增加一个“观察者增删缓存”队列。
三、修改attach函数和detach函数。用户请求增删迭代子的时候，如果系统正在迭代过程中，那么先将增删请求保存在观察者增删缓存队列中，等待迭代完成之后再将这些请求付诸实现。如果不处于迭代过程中，直接操作
观察者队列可以了。

本文转自我的blog：
http://blog.csdn.net/skyMountain/archive/2006/09/19/1245058.aspx

[jiang_nan1981]

mark

[shan_ghost]

楼主不是真正的CPPer...^_^

C/C++的哲学是：在不（潜在地）多耗费一个时钟周期的前提下，给用户提供更多的功能；

而java等语言的哲学则是：在保证功能、保证用户使用体验的前提下，尽量节省更多的时钟周期。

java认为它的用户都是工程师，C/C++认为它的用户都是算法大师^_^


比如LIST模板，提供一个“任何时间都能保证有效的ITERATOR”很容易；但如果用户只是拿它当普通固定大小的数组用，这个保证就只是在白白浪费时钟周期。
这点浪费，java可以不在乎，C/C++在乎。


所以，记住C/C++库的设计原则：如果最终算法是既不牺牲空间、又不牺牲时间的完美实现，提供它；如果最终算法在某方面有所折中，那么，提供它的完美的“零件”但不提供它（或者，既提供这些零件，也提供完美的折中版本）；如果最终算法连“完美的零件”都不存在，不提供它。

所以，C++提供auto_ptr，但不提供通用的垃圾收集机制——虽然写一个也不难。

对于你的代码：要么自己扩展标准LIST，让它能保证ITERATOR总是有效；要么干脆以const关键字限制用户，使他不可能修改它。

[redleaves]

程序的终极目标只不过是"正确,健壮,快速"而已.用适合的方法做适合的事,不要这种无谓的事上花太多力气.

[zeusever]

to:shan_ghost
好像可选择的垃圾收集已经在0x的列表中了吧。。。


不过我觉得这个问题不应该是c++ 才有的吧。。实际上是你的观察者列表被脏读了哦。

[heguodong]

作者的意见我不赞同,我觉得设计模式在语言面前都是一样的,除非你不会设计或者不理解语言
迭代子失效的问题,是你自己编码的问题,C++语言给了你充分的自由去处理这些问题
工厂对象的问题,我认为也不是问题,对象自身删除自己不违背原则吧,那就好好看看COM对象是怎么删除的,这么自由的C++语言机制对你来讲却成了负担,那么我只能说你的C++还有限或者说设计有限

[danjiewu]

不如每次都用lstMsgListener新生成的一个副本进行迭代，稍微多一些代价，但是一定安全。

[holyfire]

attach deattch前先不要直接加入list,将申请放入另一个Command链表,等到有notify来的时候重新整理链表就可以了

我觉得这个问题是实用不当,比如说我在Notify里面Attach同一个Handler,那么就会引起死循环,Notify -> Attach -> Notify -> Attach

那么你觉得这个是模式的缺陷么

[boxban]

shan_ghost() 关于语言哲学的一段论述还是有一定道理的。
建议每个试图在比较Java 和 C++ 时对某些C++中缺失的特性进行批评时，先去仔细读一读 B.S 的《C++语言的设计和演化》

[boxban]

另外，对于你提出的个案，有一个偷懒的解决办法：

void CSubject::OnMsg(CSMSG *pMsg)
{
list<IMsgListener*>::iterator tmp;
for(list<IMsgListener*>::iterator it = m_lstMsgListener.begin();
it != m_lstMsgListener.end(); ) //将it++ 提前做掉
{
tmp = it++;
ASSERT( NULL != (*tmp) );
(*tmp) ->Notify(pMsg);
}
}

[fohoo]

只是个iterator失效的问题吧，，不要被iterator缚住，不要被C++太多的特性迷住了双眼。

listener/event，在dispatch时，，取消listener是正当，，应当实现的。。如果换个角度，从C程序员的角度思考，解决方案就一目了然。。

iterator失效而已，就不让他失效，只做个删除标记就行了。

void AddListener(IMsgListener* listener)
{
//找到Listener，delete设为true,否则加listener
}


void RemoveListener(IMsgListener* listener)
{
//找到并设Listener的delete
}

void CSubject::OnMsg(CSMSG *pMsg)
{
for(map<IMsgListener*,bool>::iterator it = m_lstMsgListener.begin();
it != m_lstMsgListener.end();)
{
ASSERT( NULL != (*it) );
if(*it.second)
{
erase(it++);
}
else
{
(*it++).first->Notify(pMsg);
}
}
}

[Aviyeah]

以下是我的看法：
list<int> l;
.....
list<int>::iterator it = l.begin();
l.erase(i++);//这里的i是一个合法的非未端迭代器，指向l内部，递增一个有郊的迭代器
但下面这样写就有问题了

list<int> l;
.....
list<int>::iterator it = l.begin();
l.erase(i);
i++;// 这时的i不是一个有效的迭代器了。
// 大部分的右边++的原型都是这样的
// const obj operator++(int);
//这说明了，这个运算符函数，i 的新值和旧值都存在于函数数体内，反回的是旧值。

[mayflowers]

对于一个纯粹的 Observer 模式来说，要求 Observer 在被 Notify 的时候不能 Unregister 自身是很有必要的。

在 Subject 分发通知的时候，复制一份或者两份 Observer 链只能从形式上解决特定应用的问题；并不能解决所有问题。复制 Observer 链只能保证 Observer 将自己删除后，Iterator 依旧可用，就此而已。但是真正的 Observer 对象可能已经不复存在。也就是说 Subject 不能再对它进行任何操作，这只是一个约定，不是一个保证。

比如说在一个异步的架构下，Subject需要分发的不是一个事件，而是一列事件，当积累了多个事件后逐次发给 Observer:

list<sharde_ptr<Event> > event_list;
list<sharde_ptr<Observer> > observer_list;
for( list<sharde_ptr<Event> >iterator iterObserver = ... ) {
for( list<sharde_ptr<Observer> >iterator iterEvent = ... ) {
(*iterObserver)->Notify(*iterEvent);
}
}

这种分发方式也许有些怪异，但是“优雅”的Observer难道要依赖于Subject分发事件的方式？
如果 Observer 在收到第一个事件后不仅仅 unregister，而且干脆自杀，结果会怎么样？

[fohoo]

我的方法只做个删除标记，可以解决Observer在notify自杀的问题

void AddListener(IMsgListener* listener)
{
//找到Listener，delete设为true,否则加listener
}


void RemoveListener(IMsgListener* listener)
{
//找到并设Listener的delete
}

void CSubject::OnMsg(CSMSG *pMsg)
{
for(list<pair<IMsgListener*,bool>>::iterator it = m_lstMsgListener.begin();
it != m_lstMsgListener.end();)
{
ASSERT( NULL != (*it) );
if(*it.second)
{
erase(it++);
}
else
{
(*it++).first->Notify(pMsg);
}
}
}

[mgphuang]

哥们，你太形而上学了。模式只给你一个思路，任何一个模式的代码对于你的程序都是无效的。对于你的一会儿程可以写出无数不会出错的解法。比如，在观察者的Notify里使用返回值来确定是否要继续传递消息，而Notify中对观察者链的修改使用异步消息的方法。等观察者链处理完成后再集中处理对观察者链的修改。

[thinkinnight]

大家的讨论真好，对于模式，我感觉也是这样，是否C++中的模式与GOF的有不同呢？

[AWolfBoy]

对于楼主的问题其实这并不是什么Observer的设计模式造成的陷阱，是楼主对使用标准库容器还不够了解，可以去参考一下Effective STL
vector/list/map/set－－在遍历迭代子的时候，如果改变容器，只要不超过其最大容量是不会使原有迭代子失效的，但如果超过了，就将重新分配容量，并把原有数据重新拷贝一份到新的容器，这样迭代子肯定是要失效
deque--使用这个容器唯一比vector有好处的地方就是在于它的内存管理方式也是连续的，但属于页面式内存管理，这个怎么理解呢，就是说在容量增长的情况下，它会重新申请一块内存，但原有的迭代子指针不会失效

有一种方法可以解决楼上的问题，而且也允许动态改变lstMsgListener
void CSubject::OnMsg(CSMSG *pMsg)
{
int nIterpos = 0;
while (nIterpos < lstMsgListener.size())
{
vector<IMsgListener*>::iterator it = m_lstMsgListener.at(nIterpos++);
ASSERT( NULL != (*it) );
(*it) ->Notify(pMsg);
}
}

以上代码需要把容器改为list才行，我觉得list相对vector来说唯一的好处也就在于内存上的消耗

[AWolfBoy]

是改为vector，一时打快了

[netren_cn]

这个问题，我在写程序时都出现过，后来就是禁止了detach就没问题了，现在想想，为什么mfc的msg也用了这个模式，但是也没这个问题，其实关键就是list的问题，自己写个特定的list就可以了。

[chon81]

我觉得问题不是设计模式或C++的问题.
设计模式本来只是提供一个设计思想, 对于具体实现和细节上要看实现者的实现.

你上面的代码问题是list的实现, 跟Observer模式是没有关系的.
Notify时list应该是一个复本, 对于这时加入新的观察者应该是在下次Notify时才有效.
如果一定要这次有效, 应该可以在添加时就Notify.

[dyf198215]

与设计模式无关哦，自己的用法问题了。

没有养成良好的习惯而已，对容器迭代循环时，最好不要对容器做增删等操作，不然问题当然多多了。