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

bvar 的全局 VarMap 锁为 pthread mutex, 可能引发死锁 #2888

Open
r-value opened this issue Feb 6, 2025 · 4 comments
Open

bvar 的全局 VarMap 锁为 pthread mutex, 可能引发死锁 #2888

r-value opened this issue Feb 6, 2025 · 4 comments

Comments

@r-value
Copy link

r-value commented Feb 6, 2025

Describe the bug (描述bug)
在从 /brpc_metrics 采集 metric 的时候, 请求由 bthread 执行, 而此时会为了锁住全局 VarMap 获取一个 pthread mutex, 当有一个用户定义的 metric 中出现了 bthread yield, 则会引起死锁.

To Reproduce (复现方法)
使用 PassiveStatus, 然后在这个 metric 的计算函数中引入一个 bthread::Mutex 的获取, 即会引起死锁. 其他的会产生 bthread yield 的同步原语均可引起同样行为.

Expected behavior (期望行为)
个人认为在这里出现 pthread mutex 是反直觉的, 尤其是在文档强调不要令 pthread mutex 和 bthread mutex 混用的情况下. 希望可以:

  • 将这个锁改为 bthread mutex, 或者
  • 在文档中做补充, 强调不要在自定义 metric 中 yield bthread

Versions (各种版本)
OS:
Compiler:
brpc:
protobuf:

Additional context/screenshots (更多上下文/截图)

@chenBright
Copy link
Contributor

在文档中做补充, 强调不要在自定义 metric 中 yield bthread

可以在文档中强调一下。

@r-value
Copy link
Author

r-value commented Feb 7, 2025

好奇这里用 pthread mutex 是有什么设计上的考量吗🤔我的比较naive的理解是 bthread mutex 也可以在纯 pthread 中安全使用

@chenBright
Copy link
Contributor

我理解的是,设计上bthread依赖bvar,所以不能用bthread mutex。

@ZhengweiZhu
Copy link

在builtin service(包括/brpc_metrics和/vars)中访问metric时,不论访问一维(对应VarMapWithLock)还是多维(对应MVarMapWithLock),这里都使用的是pthread mutex。在bthread(builtin service是在bthread中执行的)中使用pthread同步原语是不正确的,可能造成死锁的用法。
作者描述的这种情况造成死锁的原因可能是:处理brpc_metrics服务的bthread所在的pthread worker先占用了MVarMapWithLock 中的pthread mutex,随后在对mvar get_value时通过PassiveStatus自定义的计算函数访问了某个bthread锁并产生了yield。问题的关键在于该bthread唤醒后可能不在原先的pthread worker上,此时再去对MVarMapWithLock的pthread mutex进行解锁是undefined behavior。下一次再访问到该pthread mutex加锁时,由于锁仍被占用而导致死锁。

由于业务代码可能在PassiveStatus自定义的计算函数中确实有加锁保护的需求,通过在文档中强调避免这种写法,不是最优做法,或者很难避免这种死锁问题出现。

正确做法是VarMapWithLock以及MVarMapWithLock应该使用bthread锁来保护,原先的写法属于是bug。将锁改用bthread锁后问题应该能解决。但在bazel编译的情况下,bthread模块由于使用了bvar,编译会依赖bvar模块,而bvar模块要想使用bthread锁又需依赖bthread模块,造成循环依赖,要解决循环依赖得对相关模块结构进行重构,改动不小 😞

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

No branches or pull requests

3 participants