cocos2d / cocos2d-x

Cocos2d-x is a suite of open-source, cross-platform, game-development tools utilized by millions of developers across the globe. Its core has evolved to serve as the foundation for Cocos Creator 1.x & 2.x.
https://www.cocos.com/en/cocos2d-x
18.22k stars 7.06k forks source link

3.15 socketio disconnect socket will crash! High probability, #17965

Open huangapple opened 7 years ago

huangapple commented 7 years ago

cocos's tests/cpp-tests will crash. 3.15 socketio disconnect socket will crash! High probability,

continuemycoding commented 7 years ago

@minggo @dumganhar 3.15版本的WebSocket关闭时有死锁问题,出现概率非常非常高。我是这样修复的,有更好的方案? 修复WebSocket死锁问题的补丁文件截图

minggo commented 7 years ago

I have asked @dumganhar to take a look.

dumganhar commented 7 years ago

@gdboy WebSocket::close is a synchronous method. Therefore, it has to wait the WebSocket thread to receive closed event. So your patch changes the logic of WebSocket::close. Currently, we provide WebSocket::closeAsync interface for closing websocket asynchronously.

Could you tell us how did you reproduce the dead lock issue ? Is there an easy way to reproduce it?

dumganhar commented 7 years ago

BTW, I think dead lock should not cause crash issue. So @gdboy 's issue isn't relative to this one.

continuemycoding commented 7 years ago

@dumganhar 在截图中的这个测试样例,只要open close就会出现死锁,我测试来看,这个bug三次至少出现一次。死锁时界面卡死不动,跟崩溃是一样的。我那样改也是有用锁进行同步等待,我看你们原先的代码是有逻辑问题。

image

continuemycoding commented 7 years ago

@dumganhar 英文只有阅读的能力,中文回复勿怪。

dumganhar commented 7 years ago

@gdboy , but your _closeCondition.wait() is in the onConnectionClosed which is invoked in sub-thread rather than cocos's thread. I will make no sense. And the WebSocket::close will return without waiting websocket instance to be closed.

continuemycoding commented 7 years ago

@dumganhar onConnectionClosed 顾名思义应该是在close完成的时候回调的?onConnectionClosed 应该是等待 WebSocket::close 才对吧?而且那样改之后确实解决了死锁问题,之前死锁大概率发生,需要等个十几秒至几十秒才能满足等待锁的条件。

dumganhar commented 7 years ago

@gdboy websocket close event will be triggered by websocket server so it probably should wait some time for receive that event. You could use WebSocket::closeAsync for close the websocket asynchronously. Yep, yep, SocketIO currently has some issues that it's hard to be fixed. We need to do some refactor on that. and SocketIO::disconnect should invoke WebSocket::closeAsync instead for not blocking the user's thread.

continuemycoding commented 7 years ago

@dumganhar 如果不那样解决,那这个死锁问题怎么解决好?没试过WebSocket::closeAsync,但是WebSocket::close确实存在大概率死锁问题。

continuemycoding commented 7 years ago

@dumganhar disconnect里面调用的不是WebSocket::closeAsync而是WebSocket::close,原先代码死锁几乎必现。 image image