-
Notifications
You must be signed in to change notification settings - Fork 266
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
在没有调度器的情况下,有爆栈问题。 #404
Comments
hmmm 我没看到相关的文字,你可以扩展下吗? |
抱歉,因为是手机上提的问题,不是特别方便复制app里面的内容,我截图下来 其中测试代码 https://gist.github.com/Jackarain/67c787a8eec214e5a06bc27ddf304ef8 |
看到了,刚才网页里我搜索没搜到,是因为被折叠了。 我感觉问题不太大,因为按 async_simple 的设计,我们是不希望用户直接调用 std::coroutine_handle::resume 这么原始的原语的。 我大概看了下 ucoro 里的那个例子,感觉实现比较复杂了。我觉得这个 trade off 可能不一定值得。 |
首先吧,我认为如果真的不希望用户调用底层的接口,应该在设计上就杜绝这种可能,也就不能暴露协程handle给用户,比如像asio的协程支持,就是用户层无法访问协程handle。 既然用户能访问,而且示例中也有这类的用法,也属于十分正常的用法,那么解决这个问题是非常有必要的,这也是库的健壮性问题。 当然这是我的建议,如果你并不认同,不打算处理,这是你的代码,决定权在你。 谢谢。 |
一方面只要用户可以自定义 awaiter 我们就没法隐藏 coroutine handle 吧,另一方面上面的例子属于 demo example,其实不属于 async_simple 的核心设计。我还是希望我们可以通过一致的设计来降低用户和实现者的 overhead。当然我们之后会找时间改下 demo。也感谢你的反馈。 |
其实这个问题解决起来并不困难。也不是属于什么解决了这个问题就破坏了其他代码的问题。 |
我本来其实也没有想在公开的地方发 issue 以致招人记恨,主要是昨天在知乎上讨论到了这个崩溃问题,其实我是有点意识到会不会遭到记恨,措辞已经非常注意了,尽量把问题的主要责任往协程提案作者身上推(他也确实有一定的责任吧)。 但是看到好几个人都说在推荐用这个项目,那么我明明非常清楚的知道这个项目没有解决这个问题,我也就按赖不住给他们讲述了这个问题,最后想想还是提了 issue,希望能避免别人踩坑。 当然我在提的时候就预感到作者不会理会我提的问题,因为我的这种公开场合讨论这个问题,已然有点冒犯到了他们(国内大多项目都非常敏感),即便是最后他们会去修,也不会说是在我提的 issue 的基础上去修的。 你说的没错,这个问题对于实现过协程的人,了解这个问题的朋友来说,其实并不是什么影响代码封装结构,主要就是 await_suspend 的实现上,应该还不至于破代码结构。 当然代码是人家的,决定权在人家,即便是随手能改的东西,外人也很难干预。 |
hmmm 不明白你们在说什么。正常的技术讨论有什么好记恨的。在公开场合讨论技术问题本来也比私下讨论更好。 这个问题本身我确实觉得不算问题,一方面调度器是设计的核心,syncAwait 在文档里说了不推荐使用,主要是为了测试。另一方面,那个 AwaiterCallback 是 demo 里的东西,不算 async_simple 里的代码里,我觉得找时间改掉就好了。 我最近时间在 async_simple 上的时间确实比较少了,你们觉得这个比较重要的话,也可以发 PR 来改的。 |
@Jackarain 我想你是误解了,async_simple 是开源的,不是封闭的,事实上社区有很多人都在贡献代码的,欢迎提PR,我们稍晚点也可以提一个PR来修复。 我reopen 一下这个issue,直到问题修复为止。 |
async_simple本身的设计有这个问题么?我看了下好像只有demo的这个例子是这样的。 |
本质是 demo 的问题,async_simple 按设计来说还是想避免回调的 |
是 微软的锅。提交的第一个版本是带 bug 的。而 async_simple 是按第一个版本设计的。要修正需要按微软提交的第三个版本设计。略有改动。编译器为了不破坏兼容性,是把 bug 版本也兼容了的。 |
这里的第 N 个版本指什么? |
jackarain 发的评论区里有引用一篇 微软的人写的解释来龙去脉的文章。 |
这个吧? https://lewissbaker.github.io/2020/05/11/understanding_symmetric_transfer Lewis Baker 不是微软的,原先的行为也谈不上 bug。至于 symmetric transfer 这个特性,咬文嚼字地说其实不算标准的一部分。虽然这可能算事实上的隐式标准,但 symmetric transfer 对硬件是有一定要求的,在 X86 和 Arch 之外的部分硬件上其实是没法实现的,这也是他不是标准一部分的原因。 Returning std::noop_coroutine 相比 return void 会多两次内存调用,所以也不太能说 return void 的 await_suspend 是 bug。另外虽然按照规定在 |
首先,await_suspend 返回 void 严格来说是不标准的。因为那是属于微软提交的第一版。是收录在 c++20 : ts 里的。而不是正式标准里。 |
|
初步想法是:
|
hmmm 说标准还是按正式版本来吧,既然正式版本收录了然后说这个不是标准这个说法太奇怪了 |
我倾向不添加 callback awaiter。在等待什么结束后执行这本就是 co_await Lazy 的语义。
之前提到过,应该让这几个组件通过 executor 进行调度而非直接 resume |
但没有调度器的情况下,mutex还是会有问题。我的想法是,可能要看看如何避免mutex::unlock重入时立即resume协程,而是延迟到上层的unlock来resume协程。 |
但是用户需要一个能够suspend/resume协程的工具。其实目前已经有了,就是async_simple::future/promise。 这个组件即使不添加调度器,也不会因为在suspend中resume而爆栈,因为future awaiter在suspend的时候不可能立即执行Continuation,从而避免了这个问题。 因此或许可以这么说,对非专家用户来说,考虑使用async_simple::future/async_simple::promise来封装回调函数,而不是demo中的callback_awaitor,则可以避免这个问题。至于用户自定义的awaitor,取决于用户自己的设计了。 此外,假如一个future从wait中resume后,会调用另外一个promise的set_value,然后唤醒另外一个future(并不断这样递归下去),那么在没有调度器的情况下依然可能会爆栈,但是这个问题相对来说比Mutex轻一些(大量协程争抢同一个mutex很常见,但是promise这种情况相对没那么容易出现),不过或许也需要像mutex一样做一些处理。 |
hmm 我一开始的想法是所有任务都应该绑定调度器的。这个地方代码里没写死确实导致后续比较烦 |
抢占mutex爆栈的情况是在有调度器的情况下发生的么, |
无调度器。 |
如果要在无调度器的情况下使用,你看下 https://github.com/avplayer/ucoro 这个库,它没有爆栈问题。 |
我的想法是,mutex类添加一个变量来检测unlock的重入次数,如果重入次数过多,则将协程的handle保存到mutex内,延迟到上层unlock函数resume,这样就能消除尾调用了。 |
是这样的。他这个是协程的 mutex , 用在协程上的。 然后,有十万个协程都在 co_await mutex.lock(); 那么,有一个协程 执行 mutex.unlock(), unlock 一直到十万个协程的 unlock 都跑完,栈才会清。 |
demo里面的callbak waiter只是专门给asio 调度用的,不是暴露给用户立即调用的,如果需要立即调用且不爆栈的话再加一个支持立即调用callback waiter就好了,实现起来并不复杂。 我建议在demo里加一个支持立即调用的callback waiter并附上测试代码,这样不必纠结于callback waiter的问题了。 |
我意见是不要去优化不应该存在的东西。还是亡羊补牢比较好。看下现在怎么提示用户不要在不绑定调度器的情况下,开启协程工作。 |
看上面我的分析。这个 unlock 就算绑定了调度器,该爆栈还是得爆栈 |
mutex的unlock里resume的是这个协程 |
同意,不建议在现在的版本上强改,可以单独开个分支,重新设计一下无调度器版本的实现,不然代码会很乱的。 比如说实现一个只在当前上下文跑的调度器,语义上和现在的调度器保持一致,但实际上只在当前上下文跑(调度点循环resume ready队列,schedule就是将coroutine handle推进队列里),这样可以避开所有的立即resume导致爆栈的问题,强制挂起协程回到调度点参加调度,这个过程会清栈。 如果这样可行的话就不用case by case的解决爆栈问题了。 |
``
是的,我也是这个思路
|
你好,关于具体问题的描述,是在知乎的一篇文章的评论中
https://zhuanlan.zhihu.com/p/12979867707?utm_psn=1852660783080665089
在评论区里提到了 async_simple 在没有调度器的特定情况下的爆栈问题
The text was updated successfully, but these errors were encountered: