You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by GitBox <gi...@apache.org> on 2020/08/07 09:26:42 UTC

[GitHub] [incubator-brpc] bengol opened a new pull request #1200: reduce sampler's mutex scope

bengol opened a new pull request #1200:
URL: https://github.com/apache/incubator-brpc/pull/1200


   Change-Id: I7e413acedc8dfb93eae5c08243f0ff3e484f7059
   
   问题: 线上环境中,进程退出时,hang在sampler->destroy()中的_mutex lock上,持有该_mutex的线程已经不再使用该锁且在正常运行,没有分析出发生路径
   
   改动原因: _used变量无需_mutex保护,减少_mutex的范围来规避该问题。同时stop变量跨越了线程,需要atomic


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] bengol commented on pull request #1200: reduce sampler's mutex scope

Posted by GitBox <gi...@apache.org>.
bengol commented on pull request #1200:
URL: https://github.com/apache/incubator-brpc/pull/1200#issuecomment-727818601


   @zyearn 辛苦看下这个?理论分析上看是没有风险的


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] jamesge commented on a change in pull request #1200: reduce sampler's mutex scope

Posted by GitBox <gi...@apache.org>.
jamesge commented on a change in pull request #1200:
URL: https://github.com/apache/incubator-brpc/pull/1200#discussion_r525761185



##########
File path: src/bvar/detail/sampler.cpp
##########
@@ -147,31 +147,28 @@ void SamplerCollector::run() {
 
     butil::LinkNode<Sampler> root;
     int consecutive_nosleep = 0;
-    while (!_stop) {
+    while (!_stop.load(butil::memory_order_relaxed)) {
         int64_t abstime = butil::gettimeofday_us();
         Sampler* s = this->reset();
         if (s) {
             s->InsertBeforeAsList(&root);
         }
-        int nremoved = 0;
-        int nsampled = 0;
+
         for (butil::LinkNode<Sampler>* p = root.next(); p != &root;) {
             // We may remove p from the list, save next first.
             butil::LinkNode<Sampler>* saved_next = p->next();
             Sampler* s = p->value();
-            s->_mutex.lock();
-            if (!s->_used) {
-                s->_mutex.unlock();
+            if (s->_used.load(butil::memory_order_acquire)) {
+                BAIDU_SCOPED_LOCK(s->_mutex);

Review comment:
       这个_mutex也不太必要了,可以删掉

##########
File path: src/bvar/detail/sampler.cpp
##########
@@ -202,9 +199,7 @@ void Sampler::schedule() {
 }
 
 void Sampler::destroy() {
-    _mutex.lock();
-    _used = false;
-    _mutex.unlock();
+    _used.store(false, butil::memory_order_release);

Review comment:
       需要在注释里举一个场景说明一下为什么要release语意




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org