[PR] Fix potential bvar deadlock by running describe()/dump() outside the global VarMap lock (brpc)

["chenBright (via GitHub)" <gi...@apache.org>]

chenBright opened a new pull request, #3470:
URL: https://github.com/apache/brpc/pull/3470

   ### What problem does this PR solve?
   
   Issue Number: resolve #2888 
   
   Problem Summary:
   
   bvar's global `VarMap` is guarded by a pthread mutex. `Variable::describe_exposed()`
   (and `describe_series_exposed()`, `dump_exposed()`, plus the multi-dimension
   `MVariableBase` counterparts) used to invoke `var->describe()` while holding that lock. 
   For `PassiveStatus`, `describe()` runs a user-provided callback; if the callback yields 
   the bthread (e.g. by acquiring a `bthread::Mutex`), the pthread mutex is never released 
   and the process deadlocks.
   
   ### What is changed and the side effects?
   
   Changed:
   
   Run user callbacks OUTSIDE the global map lock via a small indirection handle:
   
   - New `bvar/detail/exposed_ref.h`: `ExposedRef<T>` (a reference-counted handle
     guarding an exposed object). It uses `butil::Mutex` + `butil::ConditionVariable`.
   - `describe_exposed()` / `describe_series_exposed()` / `get_exposed()`: under the
     map lock they now only `seek` + `acquire()` (ref-count +1, serialized with `hide()`'s 
     erase); the lock is released, `describe()` is called outside the lock, then `release()`.
   - `hide()` now also invalidates the handle and blocks (`hide_and_wait()`) until
     all in-flight readers finish, so a Variable cannot be destroyed while a concurrent
     `describe()` is still using it. Each `expose()` rebuilds a fresh handle (the old one
     is single-use once hidden).
   - `MVariableBase::describe_exposed()` / `dump_exposed()` get the same treatment;
     `dump()` in particular is moved outside the lock because `Dumper` is a
     user-overridable interface that may yield.
   - Since callbacks no longer run under the lock, the recursive `VarMap` mutex is no
     longer needed and is reverted to a plain mutex (now consistent with `MVarMap`).
   
   Side effects:
   - Performance effects:
   
   - Breaking backward compatibility: 
   
   ---
   ### Check List:
   - Please make sure your changes are compilable.
   - When providing us with a new feature, it is best to add related tests.
   - Please follow [Contributor Covenant Code of Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md).


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

Re: [PR] Fix potential bvar deadlock by running describe()/dump() outside the global VarMap lock (brpc)

["chenBright (via GitHub)" <gi...@apache.org>]

chenBright merged PR #3470:
URL: https://github.com/apache/brpc/pull/3470


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

Re: [PR] Fix potential bvar deadlock by running describe()/dump() outside the global VarMap lock (brpc)

["wwbmmm (via GitHub)" <gi...@apache.org>]

wwbmmm commented on PR #3470:
URL: https://github.com/apache/brpc/pull/3470#issuecomment-5377848898

   LGTM


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