go-eden / routine

Provides some convenient API, includes Goid(), GetG() and LocalStorage, which is a goroutine's local storage, just like ThreadLocal in other languages.
MIT License
114 stars 20 forks source link

提了一个pr #15

Closed funbytes closed 2 years ago

funbytes commented 2 years ago

提了一个pr,使用go的地方加了recover的保护,要不一个panic就可能导致整个进程退出

sisyphsu commented 2 years ago

routine作为一个底层库,它并不知道panic会对系统造成什么样的破坏,此时主动recover就是越俎代庖,可能引发更加严重的未知问题。

协程panic时,进程退出是golang的设计,上层业务开发者可以在确保安全的前提下,自己进行recover处理。

funbytes commented 2 years ago

routine作为一个底层库,底层库自己启动的goroutine因为调用自己的register如果出错了没有catch panic导致进程退出的话,这样不好吧?

sisyphsu commented 2 years ago

routine作为一个底层库,底层库自己启动的goroutine因为调用自己的register如果出错了没有catch panic导致进程退出的话,这样不好吧?

请问你在使用过程中遇到register出现的panic吗? 我需要评估下出现的是什么panic,以及如何补偿。

funbytes commented 2 years ago

目前还没有在生产环境中使用routine,不过以我们多年使用golang的经验,曾经好几次在线上遇到过使用的第三方库因为没有对go func(){} 做recover保护导致在极端情况或者压力陡增的情况下进程退出,遇到这种情况非常难查出问题,所以现在我们使用第三方库都会首先看go有没有做保护,即使recover后直接打印日志,然后fatal退出进程,也比什么都不做要强很多,起码可以查到是在代码什么地方退出的

sisyphsu commented 2 years ago

recover后直接打印日志,然后fatal退出进程,也比什么都不做要强很多,起码可以查到是在代码什么地方退出的

这个确实会更加稳妥一些,我完善一下register函数内部的panic日志

funbytes commented 2 years ago

加油,这些都是被坑过好多回以后的血泪经验啊.....

sisyphsu commented 2 years ago

考虑的register失败只会导致协程的local变量脱离GC,其内存泄漏的风险及影响范围比较可控,所以我在新版本1.0.1中的register()函数中增加了recover,万一出现问题也只是输出errmsg,避免kill掉当前进程。

谢谢你的建议与PR~