search

xiehuc / pidgin-lwqq

a pidgin plugin based on lwqq, a excellent safe useful library for webqq protocol
GNU General Public License v3.0
660 stars 185 forks source link

竟态条件 #631

Closed yanglimingcn closed 9 years ago

yanglimingcn commented 9 years ago

您好，我最近在用kopete qq频繁崩溃，查看了代码，看到一处竟态条件没有保护，可能会导致内存异常，因为也只是在工作之余看了一周的时间，不知道我的问题是否确实是个问题，还请帮忙分析一下。 问题描述如下： 线程1执行： LwqqAsyncEvset* set = lwqq_async_evset_new(); ev = lwqq_info_get_single_long_nick(lc, buddy); lwqq_async_evset_add_event(set, ev); ev = lwqq_info_get_level(lc, buddy); lwqq_async_evset_add_event(set, ev);

线程2执行： check_multi_info里面的 //执行完成时候的回调 if(lwqq_client_valid(lc)) lwqq_client_dispatch(lc,C(p,async_complete,conn)); 时候，会把set释放掉。

这个时候线程1和线程2存在竟态条件，如果线程1执行完第一个add_event，线程2把set释放了，线程1再执行第二个add_event，这样它将引用了一个释放掉的内存空间，这样就会导致程序崩溃。

请看看是否会出现这个的问题呢？如果有应该如何使用该库才能避免此类问题出现呢？

xiehuc commented 9 years ago

因为网络通信的时间肯定要比本地执行代码的时间慢很多，因此不会说执行完第一个add event就马上complete把set释放了。kopete qq可能是用的早期lwqq库没有更新。崩溃的话最好还是运行gdb在哪出问题的能很快定位到。

El Psy Congroo

在 2015年4月7日，09:41，sunshiney notifications@github.com 写道：

您好，我最近在用kopete qq频繁崩溃，查看了代码，看到一处竟态条件没有保护，可能会导致内存异常，因为也只是在工作之余看了一周的时间，不知道我的问题是否确实是个问题，还请帮忙分析一下。 问题描述如下： 线程1执行： LwqqAsyncEvset* set = lwqq_async_evset_new(); ev = lwqq_info_get_single_long_nick(lc, buddy); lwqq_async_evset_add_event(set, ev); ev = lwqq_info_get_level(lc, buddy); lwqq_async_evset_add_event(set, ev);

线程2执行： check_multi_info里面的 //执行完成时候的回调 if(lwqq_client_valid(lc)) lwqq_client_dispatch(lc,C(p,async_complete,conn)); 时候，会把set释放掉。

这个时候线程1和线程2存在竟态条件，如果线程1执行完第一个add_event，线程2把set释放了，线程1再执行第二个add_event，这样它将引用了一个释放掉的内存空间，这样就会导致程序崩溃。

请看看是否会出现这个的问题呢？如果有应该如何使用该库才能避免此类问题出现呢？

— Reply to this email directly or view it on GitHub.

yanglimingcn commented 9 years ago

您好，我这个kopete qq用的lwqq库是0.4.2版本。出现这个崩溃，我用gdb调试过，分析看也确实因为这个竟态条件造成的 Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7ffff7fa3880 (LWP 27608)] 0x00007fffdceea1a5 in vp_link (head=0xe5ba98, elem=0x7fffffffd0f0) at /usr/src/packages/SOURCES/lwqq-0.4.2/lib/vplist.c:101 101 while(cmd->next) (gdb) bt

0 0x00007fffdceea1a5 in vp_link (head=0xe5ba98, elem=0x7fffffffd0f0) at /usr/src/packages/SOURCES/lwqq-0.4.2/lib/vplist.c:101

1 0x00007fffdceee1b5 in lwqq_async_add_evset_listener (evset=0xe5ba30, cmd=...) at /usr/src/packages/SOURCES/lwqq-0.4.2/lib/async.c:256

2 0x00007fffdd16984a in QQAccount::ac_login_stage_3 (this=0xaf0870, lc=lc@entry=0xcba750) at /home/limingy/Sources/kopete-qq-build/qqaccount.cpp:1569

3 0x00007fffdd169b1a in QQAccount::slotReceivedInstanceSignal (this=this@entry=0xaf0870, cb=...) at /home/limingy/Sources/kopete-qq-build/qqaccount.cpp:334

4 0x00007fffdd169f30 in QQAccount::qt_static_metacall (_o=0xaf0870, _c=, _id=23, _a=0x7fffcc05eaf0)

at /home/limingy/Sources/kopete-qq-build/build/qqaccount.moc:138

5 0x00007ffff38c411e in QObject::event(QEvent_) () from /usr/lib64/libQtCore.so.4

6 0x00007ffff27ae8ac in QApplicationPrivate::notifyhelper(QObject, QEvent_) () from /usr/lib64/libQtGui.so.4

7 0x00007ffff27b4e70 in QApplication::notify(QObject, QEvent) () from /usr/lib64/libQtGui.so.4

8 0x00007ffff431518a in KApplication::notify(QObject, QEvent) () from /usr/lib64/libkdeui.so.5

9 0x00007ffff38ac0ad in QCoreApplication::notifyInternal(QObject, QEvent) () from /usr/lib64/libQtCore.so.4

10 0x00007ffff38af0ff in QCoreApplicationPrivate::sendPostedEvents(QObject_, int, QThreadData*) () from /usr/lib64/libQtCore.so.4

11 0x00007ffff38d9493 in ?? () from /usr/lib64/libQtCore.so.4

12 0x00007fffed374316 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0

13 0x00007fffed374668 in ?? () from /usr/lib64/libglib-2.0.so.0

14 0x00007fffed37470c in g_main_context_iteration () from /usr/lib64/libglib-2.0.so.0

15 0x00007ffff38d8d76 in QEventDispatcherGlib::processEvents(QFlagsQEventLoop::ProcessEventsFlag) () from /usr/lib64/libQtCore.so.4

16 0x00007ffff284b936 in ?? () from /usr/lib64/libQtGui.so.4

17 0x00007ffff38aad0f in QEventLoop::processEvents(QFlagsQEventLoop::ProcessEventsFlag) () from /usr/lib64/libQtCore.so.4

18 0x00007ffff38ab005 in QEventLoop::exec(QFlagsQEventLoop::ProcessEventsFlag) () from /usr/lib64/libQtCore.so.4

19 0x00007ffff38b013b in QCoreApplication::exec() () from /usr/lib64/libQtCore.so.4

20 0x00000000004157de in main (argc=1, argv=0x7fffffffdab8) at /usr/src/packages/SOURCES/kopete-14.12.3/kopete/main.cpp:105

本来线程1新创建的evset cmd字段是空的，看上边的信息 vp_link里面cmd->next是0x58，但这个地址是错误的地址，所以程序就崩溃了。 您看有没有什么方法解决这个问题呢，pidgin的倒是一直很正常。

yanglimingcn commented 9 years ago

在login_stage_3函数里面会在多个循环里面调用add_event,如果qq好友很多，这种竟态条件就比较容易出现。您看是不是会有问题呢？

xiehuc commented 9 years ago

擦，你那个kopete qq的源代码在哪里下？我直接看下它源码好了。

El Psy Congroo

在 2015年4月7日，10:27，sunshiney notifications@github.com 写道：

您好，我这个kopete qq用的lwqq库是0.4.2版本。出现这个崩溃，我用gdb调试过，分析看也确实因为这个竟态条件造成的 Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7ffff7fa3880 (LWP 27608)] 0x00007fffdceea1a5 in vp_link (head=0xe5ba98, elem=0x7fffffffd0f0) at /usr/src/packages/SOURCES/lwqq-0.4.2/lib/vplist.c:101 101 while(cmd->next) (gdb) bt

0 0x00007fffdceea1a5 in vp_link (head=0xe5ba98, elem=0x7fffffffd0f0) at /usr/src/packages/SOURCES/lwqq-0.4.2/lib/vplist.c:101

1 0x00007fffdceee1b5 in lwqq_async_add_evset_listener (evset=0xe5ba30, cmd=...) at /usr/src/packages/SOURCES/lwqq-0.4.2/lib/async.c:256

2 0x00007fffdd16984a in QQAccount::ac_login_stage_3 (this=0xaf0870, lc=lc@entry=0xcba750) at /home/limingy/Sources/kopete-qq-build/qqaccount.cpp:1569

3 0x00007fffdd169b1a in QQAccount::slotReceivedInstanceSignal (this=this@entry=0xaf0870, cb=...) at /home/limingy/Sources/kopete-qq-build/qqaccount.cpp:334

4 0x00007fffdd169f30 in QQAccount::qt_static_metacall (_o=0xaf0870, _c=, _id=23, _a=0x7fffcc05eaf0)

at /home/limingy/Sources/kopete-qq-build/build/qqaccount.moc:138

5 0x00007ffff38c411e in QObject::event(QEvent) () from /usr/lib64/libQtCore.so.4

6 0x00007ffff27ae8ac in QApplicationPrivate::notify_helper(QObject, QEvent) () from /usr/lib64/libQtGui.so.4

7 0x00007ffff27b4e70 in QApplication::notify(QObject, QEvent) () from /usr/lib64/libQtGui.so.4

8 0x00007ffff431518a in KApplication::notify(QObject, QEvent) () from /usr/lib64/libkdeui.so.5

9 0x00007ffff38ac0ad in QCoreApplication::notifyInternal(QObject, QEvent) () from /usr/lib64/libQtCore.so.4

10 0x00007ffff38af0ff in QCoreApplicationPrivate::sendPostedEvents(QObject, int, QThreadData*) () from /usr/lib64/libQtCore.so.4

11 0x00007ffff38d9493 in ?? () from /usr/lib64/libQtCore.so.4

12 0x00007fffed374316 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0

13 0x00007fffed374668 in ?? () from /usr/lib64/libglib-2.0.so.0

14 0x00007fffed37470c in g_main_context_iteration () from /usr/lib64/libglib-2.0.so.0

15 0x00007ffff38d8d76 in QEventDispatcherGlib::processEvents(QFlagsQEventLoop::ProcessEventsFlag) () from /usr/lib64/libQtCore.so.4

16 0x00007ffff284b936 in ?? () from /usr/lib64/libQtGui.so.4

17 0x00007ffff38aad0f in QEventLoop::processEvents(QFlagsQEventLoop::ProcessEventsFlag) () from /usr/lib64/libQtCore.so.4

18 0x00007ffff38ab005 in QEventLoop::exec(QFlagsQEventLoop::ProcessEventsFlag) () from /usr/lib64/libQtCore.so.4

19 0x00007ffff38b013b in QCoreApplication::exec() () from /usr/lib64/libQtCore.so.4

20 0x00000000004157de in main (argc=1, argv=0x7fffffffdab8) at /usr/src/packages/SOURCES/kopete-14.12.3/kopete/main.cpp:105

本来线程1新创建的evset cmd字段是空的，看上边的信息 vp_link里面cmd->next是0x58，但这个地址是错误的地址，所以程序就崩溃了。 您看有没有什么方法解决这个问题呢，pidgin的倒是一直很正常。

— Reply to this email directly or view it on GitHub.

yanglimingcn commented 9 years ago

源码地址：http://git.oschina.net/zhjun5337/kopete-qq.git，这个代码那个人好久没维护了，我想改一些bug，使用它，kde环境下kopete感受还是比pidgin好很多的。 我的开发环境是openSUSE13.2,上边的代码编译时候依赖您的liblwqq0，使用zypper 安装的。

xiehuc commented 9 years ago

那你改的代码我能从哪里下载到呢， 不排除这里有竞争的问题，但是要改的话就意味着一些底层的设计需要修改。所以我想自己调试一下，看看该怎么解决。

这里只是用锁保护了evset的reference count的一致性，要是说好友特别多的话，导致evset的reference 提前减到0了好像是有可能，不过对于我的话还是比较难想象。

sunshiney notifications@github.com于2015年4月7日星期二写道：

源码地址： http://git.oschina.net/zhjun5337/kopete-qq.git，这个代码那个人好久没维护了，我想改一些bug，使用它，kde环境下kopete感受还是比pidgin好很多的。 我的开发环境是openSUSE13.2,上边的代码编译时候依赖您的liblwqq0，使用zypper 安装的。

— Reply to this email directly or view it on GitHub https://github.com/xiehuc/pidgin-lwqq/issues/631#issuecomment-90357294.

yanglimingcn commented 9 years ago

嗨，我现在只是把lwqq_async_event_finish代码里面的lwqq_async_evset_free(internal->host_lock);注释掉了，这样的话会导致内存泄露，但是不会崩溃了，这说明此处是有问题的。 就是因为我看到如果修改此问题会涉及到底层的变动，所以我还没动手改，先联系一下你，先确定是否有这种现象，你更熟悉你的代码，不知道是否有很容易的解决方案，我还没想出比较理想的方法修改此问题呢。

yanglimingcn commented 9 years ago

因为原始代码那个兄弟不维护了，我在github上放了代码https://github.com/limingy/kopete-lwqq.git回头我在这里修改。

xiehuc commented 9 years ago

ok，我会尽快想想办法的。

Liming Yang notifications@github.com于2015年4月7日星期二写道：

因为原始代码那个兄弟不维护了，我在github上放了代码 https://github.com/limingy/kopete-lwqq.git回头我在这里修改。

— Reply to this email directly or view it on GitHub https://github.com/xiehuc/pidgin-lwqq/issues/631#issuecomment-90444230.

yanglimingcn commented 9 years ago

嗨，我有个思路，能解决这个问题，现在稍作测试，觉得还可以，请参考。 lwqq里面新增加两个方法。 LWQQ_EXPORT LwqqAsyncEvset* lwqq_async_evsetnew2() { LwqqAsyncEvset* l = smalloc0(sizeof(LwqqAsyncEvset)); pthread_mutex_init(&l->lock,NULL); pthread_cond_init(&l->cond,NULL); pthread_mutex_lock(&l->lock); ++l->ref_count; pthread_mutex_unlock(&l->lock); return (LwqqAsyncEvset*)l; }

LWQQ_EXPORT void lwqq_async_evsetfree2(LwqqAsyncEvset* set) { if(!set) return; LwqqAsyncEvset* l = (LwqqAsyncEvset_*) set; pthread_mutex_lock(&l->lock); --l->ref_count; pthread_mutex_unlock(&l->lock); }

调用的时候如下： LwqqAsyncEvset* set = lwqq_async_evset_new2(); //添加各种事件逻辑 ev = lwqq_info_get_single_long_nick(lc, buddy); lwqq_async_evset_add_event(set, ev); ev = lwqq_info_get_level(lc, buddy); lwqq_async_evset_add_event(set, ev); ... //添加回调函数逻辑 lwqq_async_add_evset_listener(set, C(p,cb_login_stage_f,lc)); lwqq_async_evset_free2(set);

这样整个evset再添加完成各种事件前是一个事务，这个事务操作过程中不会被释放。 我不知道lwqq的代码在哪里能下载。

yanglimingcn commented 9 years ago

LWQQ_EXPORT void lwqq_async_evsetfree2(LwqqAsyncEvset* set) { if(!set) return; LwqqAsyncEvset* l = (LwqqAsyncEvset_*) set; pthread_mutex_lock(&l->lock); --l->ref_count; pthread_mutex_unlock(&l->lock); if (l->ref_count==0) { vp_do(l->cmd,NULL); lwqq_async_evset_free(set); } }

xiehuc commented 9 years ago

lwqq 的代码 在 https://github.com/xiehuc/lwqq 我刚加入了一个小patch。 evset的设计是自动的释放，所以是不需要显式的调用free的。

xiehuc commented 9 years ago

后面仔细看了下，大概我的patch和你的意思一样，只是我把free2合并进了 add_evset_listener中了。用一个ready变量。

yanglimingcn commented 9 years ago

嗨，你的处理方式很简洁，对上层的影响也很小，只是我觉得会有点问题。 因为不是每个evset都必须要求加listener，对于不加listener的evset，会导致内存泄露。 另外，看你对lwqq_async_evset_new的注释，“if it add zero event, and not trigger any thing. it would be a garbage and need freed by you.”，如果我们的evset根据一些添加event时候，条件没有满足，这样我们还是需要自己释放evset的，所以我觉得在listener后面加上释放语句还是有必要的。 你想想看，是不是应该是这个语法规则呢？

xiehuc commented 9 years ago

但是另一方面，evset的目的是为了能让执行完一批event之后能给个回调。如果你先加了一批event，然后马上free掉evset。这样在后台跑完了的所有event的时候，去触发evset的回调的时候已经被释放掉了，那还是会crash。

如果你说的条件没有满足而没有添加成功event，在evset 添加回调的时候也会判断下ref count是否为0，是的话立即回调并释放evset。所以也没有问题。那个注释可能有些老了。

El Psy Congroo

在 2015年4月8日，08:34，Liming Yang notifications@github.com 写道：

嗨，你的处理方式很简洁，对上层的影响也很小，只是我觉得会有点问题。 因为不是每个evset都必须要求加listener，对于不加listener的evset，会导致内存泄露。 另外，看你对lwqq_async_evset_new的注释，“if it add zero event, and not trigger any thing. it would be a garbage and need freed by you.”，如果我们的evset根据一些添加event时候，条件没有满足，这样我们还是需要自己释放evset的，所以我觉得在listener后面加上释放语句还是有必要的。 你想想看，是不是应该是这个语法规则呢？

— Reply to this email directly or view it on GitHub.

yanglimingcn commented 9 years ago

是这样的，我的意思是，创建evset的时候，使引用计数为1，然后加上event和listener，完成后，减少这个引用计数。减少引用计数后是否释放，要看evset里面的event的消耗了。可能我们的free会释放它并且调用callback，也可能只是减少了我们的引用计数值而已。

yanglimingcn commented 9 years ago

因为我看到lwqq程序里面有只加了event，没有加listener的地方存在，这些地方会内存泄露。

xiehuc commented 9 years ago

哦，对的，这样就没什么问题了。应该叫unref，叫free我老是想成完全回收内存。 只是这样要改的地方就比较多了。

至于event，就算没加listener，只要事件完成了就会通过finish函数，在那里释放内存了，应该没问题吧。

El Psy Congroo

在 2015年4月8日，10:39，Liming Yang notifications@github.com 写道：

是这样的，我的意思是，创建evset的时候，使引用计数为1，然后加上event和listener，完成后，减少这个引用计数。减少引用计数后是否释放，要看evset里面的event的消耗了。可能我们的free会释放它并且调用callback，也可能只是减少了我们的引用计数值而已。

— Reply to this email directly or view it on GitHub.

yanglimingcn commented 9 years ago

是的，这样就好了，没有问题的。