Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dev: fix the error of dns timeout crash #159

Merged
merged 9 commits into from
Jul 2, 2023
Merged

Conversation

maye76
Copy link
Contributor

@maye76 maye76 commented Jul 1, 2023

[zh-cn] 问题复现:#125
[en] Problem recurrence: #125

@xfangfang
Copy link
Owner

非常感谢,确实不会发生闪退了。

我准备在搞清楚dns timeout原因之后再合并这个pr。

目前还可以改善的确实是退出直播时存在小卡顿的问题,我感觉相对较好的实现是在按下退出键时屏幕转圈圈,等到可以退出时再关闭页面,这样可以保证没有任何东西在阻塞主线程。
当然,如果能做到完全无感就更好了。

@maye76
Copy link
Contributor Author

maye76 commented Jul 1, 2023

我移除了阻塞进程的部分,将控制权移出,目前出现连接等网络错误是不做处理的,如果你希望打印日志或者做一些操作(断开连接什么的),只需要检查ms_ev_ok.load(std::memory_order_acquire)返回的的bool值即可。

@xfangfang
Copy link
Owner

好的好的,非常感谢,我做完手头的事就研究这一部分~

@xfangfang xfangfang changed the base branch from dev to fix_live July 2, 2023 13:23
@xfangfang xfangfang merged commit 4add201 into xfangfang:fix_live Jul 2, 2023
@xfangfang
Copy link
Owner

非常感谢,其实现在还存在一些对主线程的阻塞,退出页面时可能随机出现最高 800 ms的阻塞,我先合并到一个其他分支了,准备好好研究一下怎么优化。

@maye76
Copy link
Contributor Author

maye76 commented Jul 2, 2023

@xfangfang 那个是设置的等待时间,在网络允许的条件下,可以降低这个时间。

  1. 我觉得可以在打开直播间或者其他什么时候,对网络延迟进行测速,根据延迟调整这个等待时间,能在减少阻塞的情况下,设置合理的等待时间。

  2. 还有就是,我现在线程里预留的发送心跳包的逻辑不合理,心跳包要求间隔20~30秒左右发送一次即可。

所以我在想,将这两个接口移出,交由上游控制

@xfangfang
Copy link
Owner

@maye174 感觉可行,那我先去搞搞别的事,这边你再完善一下?
还有两页的 todolist 没有完成~

@maye76
Copy link
Contributor Author

maye76 commented Jul 2, 2023

@xfangfang 这个很快,稍后推送到fix_live分支吗?

@xfangfang
Copy link
Owner

@maye174 对的~

xfangfang added a commit that referenced this pull request Jul 21, 2023
* Fix the live activity crash (#159)

* Improve the new interface of barrage (#164)

---------

Co-authored-by: Maye <aizaixiyq@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants