[GitHub] [incubator-brpc] zyearn commented on a diff in pull request #1817: fix additional refrerence can not release when socket is reviving

[GitBox <gi...@apache.org>]

zyearn commented on code in PR #1817:
URL: https://github.com/apache/incubator-brpc/pull/1817#discussion_r927107315


##########
src/brpc/socket.cpp:
##########
@@ -770,15 +773,22 @@ void Socket::Revive() {
 }
 
 int Socket::ReleaseAdditionalReference() {
-    bool expect = false;
-    // Use `relaxed' fence here since `Dereference' has `released' fence
-    if (_recycle_flag.compare_exchange_strong(
-            expect, true,
+    do {
+        AdditionalRefStatus expect = REF_USING;
+        if (_additional_ref_status.compare_exchange_strong(
+            expect,
+            REF_RECYLED,
             butil::memory_order_relaxed,
             butil::memory_order_relaxed)) {
-        return Dereference();
-    }
-    return -1;
+            return Dereference();
+        }
+
+        if (expect == REF_REVIVING) { // sched_yield to wait until status is not REF_REVIVING
+            sched_yield();

Review Comment:
   我不太确定这里用sched_yield()[1]是不是一个好的选择，当时有考虑用futex_wait机制实现吗
   
   [1] https://news.ycombinator.com/item?id=21960334



##########
src/brpc/socket.h:
##########
@@ -784,9 +785,17 @@ friend void DereferenceSocket(Socket*);
     // Set by SetLogOff
     butil::atomic<bool> _logoff_flag;
 
-    // Flag used to mark whether additional reference has been decreased
-    // by either `SetFailed' or `SetRecycle'
-    butil::atomic<bool> _recycle_flag;
+    enum AdditionalRefStatus {
+        REF_USING,        // additional ref is using normally
+        REF_REVIVING,     // additional ref is reviving
+        REF_RECYLED       // additional ref has benn recyled

Review Comment:
   benn -> been
   
   另外这里的注释可能需要修改一下，additional ref is reviving有点怪，主语改成socket好像更适合一点



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

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