apache · chenBright · Aug 22, 2026
diff --git a/src/brpc/input_messenger.cpp b/src/brpc/input_messenger.cpp
@@ -218,7 +218,7 @@ int InputMessenger::ProcessNewMessage(
                     << "Close " << *m << " due to unknown message: "
                     << butil::ToPrintable(m->_read_buf);
                 m->SetFailed(EINVAL, "Close %s due to unknown message",
-                                m->description().c_str());
+                             m->description().c_str());
                 return -1;
             } else {
                 LOG(WARNING) << "Close " << *m << ": " << pr.error_str();

diff --git a/src/bvar/detail/percentile.h b/src/bvar/detail/percentile.h
@@ -523,10 +523,16 @@ class Percentile {
     }
     VoidOp inv_op() const { return VoidOp(); }
 
+    // Expose the shared data carrier, so that ReducerSampler holds it instead
+    // of `this`. Sampling then keeps reading valid memory even if this
+    // Percentile is destructed before the sampler is recycled.
+    shared_combiner_type share_combiner() const { return _combiner; }
+
     // The sampler for windows over percentile.
     sampler_type* get_sampler() {
         if (nullptr == _sampler) {
             _sampler = new sampler_type(this);
+            _sampler->set_debug_name(_debug_name);
             _sampler->schedule();
         }
         return _sampler;
@@ -543,6 +549,9 @@ class Percentile {
     // This name is useful for warning negative latencies in operator<<
     void set_debug_name(const butil::StringPiece& name) {
         _debug_name.assign(name.data(), name.size());
+        if (nullptr != _sampler) {
+            _sampler->set_debug_name(_debug_name);
+        }
     }
 
 private:
@@ -591,6 +600,7 @@ class Percentile {
     sampler_type* get_sampler() {
         if (nullptr == _sampler) {
             _sampler = new sampler_type(this);
+            _sampler->set_debug_name(_debug_name);
             _sampler->schedule();
         }
         return _sampler;
@@ -610,6 +620,9 @@ class Percentile {
     // This name is useful for warning negative latencies in operator<<
     void set_debug_name(const butil::StringPiece& name) {
         _debug_name.assign(name.data(), name.size());
+        if (nullptr != _sampler) {
+            _sampler->set_debug_name(_debug_name);
+        }
     }
 
 private:

diff --git a/src/bvar/detail/sampler.cpp b/src/bvar/detail/sampler.cpp
@@ -166,9 +166,16 @@ void SamplerCollector::run() {
             Sampler* s = p->value();
             s->_mutex.lock();
             if (!s->_used) {
+                // If the sampler is still borrowed (a Window outlived the bvar
+                // it references), deleting it would leave the borrowers with a
+                // dangling pointer, so leak it on purpose. destroy() already
+                // reported the misuse.
+                const bool leaked = s->_leaked;
                 s->_mutex.unlock();
                 p->RemoveFromList();
-                delete s;
+                if (!leaked) {
+                    delete s;
+                }
             } else {
                 s->take_sample();
                 s->_mutex.unlock();
@@ -196,12 +203,17 @@ void SamplerCollector::run() {
     }
 }
 
-Sampler::Sampler() : _used(true) {}
+Sampler::Sampler() : _used(true), _nborrow(0), _leaked(false) {}
 
 Sampler::~Sampler() {}
 
 DEFINE_bool(bvar_enable_sampling, true, "is enable bvar sampling");
 
+DEFINE_bool(bvar_abort_on_sampler_still_borrowed, false,
+            "Abort when a bvar is destructed while its sampler is still "
+            "borrowed by a Window/PerSecond, namely the Window outlives the "
+            "bvar it references");
+
 void Sampler::schedule() {
     // since the SamplerCollector is initialized before the program starts
     // flags will not take effect if used in the SamplerCollector constructor
@@ -211,9 +223,53 @@ void Sampler::schedule() {
 }
 
 void Sampler::destroy() {
-    _mutex.lock();
-    _used = false;
-    _mutex.unlock();
+    int nborrow = 0;
+    std::string owner;
+    {
+        BAIDU_SCOPED_LOCK(_mutex);
+        _used = false;
+        nborrow = _nborrow;
+        if (nborrow > 0) {
+            // The owning bvar is being destructed while Window/PerSecond objects
+            // still borrow this sampler. Leak the sampler so that the borrowers
+            // keep pointing at valid memory (they just stop getting new samples),
+            // which turns a use-after-free into a bounded leak.
+            _leaked = true;
+            if (!_debug_name.empty()) {
+                owner.append("bvar`").append(_debug_name).append("`");
+            } else {
+                owner.append("An unnamed bvar");
+            }
+        }
+    }
+
+    if (nborrow <= 0) {
+        return;
+    }
+    if (FLAGS_bvar_abort_on_sampler_still_borrowed) {
+        LOG(FATAL) << "Abort because " << owner << " is destructed while "
+                   << nborrow << " Window/PerSecond still reference its"
+                      " sampler";
+    } else {
+        LOG(ERROR) << owner << " is destructed while " << nborrow
+                   << " Window/PerSecond still reference its sampler. The"
+                      " bvar referenced by a Window MUST be destructed"
+                      " AFTER that Window, see comments of Window in"
+                      " bvar/window.h. The sampler is leaked to avoid a"
+                      " dangling pointer.";
+    }
+}
+
+void Sampler::add_borrower() {
+    BAIDU_SCOPED_LOCK(_mutex);
+    ++_nborrow;
+}
+
+void Sampler::remove_borrower() {
+    BAIDU_SCOPED_LOCK(_mutex);
+    CHECK_GT(_nborrow, 0) << "remove_borrower() is called more times than "
+                             "add_borrower(), which is a bug of the caller";
+    --_nborrow;
 }
 
 }  // namespace detail

diff --git a/src/bvar/detail/sampler.h b/src/bvar/detail/sampler.h
@@ -21,6 +21,9 @@
 #define  BVAR_DETAIL_SAMPLER_H
 
 #include <vector>
+#include <string>                        // std::string
+#include <type_traits>                   // std::true_type
+#include <utility>                       // std::declval
 #include "butil/containers/linked_list.h"// LinkNode
 #include "butil/scoped_lock.h"           // BAIDU_SCOPED_LOCK
 #include "butil/logging.h"               // LOG()
@@ -57,14 +60,41 @@ class Sampler : public butil::LinkNode<Sampler> {
     // Call this function instead of delete to destroy the sampler. Deletion
     // of the sampler may be delayed for seconds.
     void destroy();
-
+
+    // Declare/undeclare that an external object borrows this sampler which is
+    // owned by another bvar. Window/PerSecond does this because it samples
+    // through the sampler of the bvar it references.
+    // If the owner is destructed while borrowers remain (namely a Window
+    // outlives the bvar it references, which violates the contract documented
+    // in bvar/window.h), destroy() reports the misuse and the sampler is
+    // deliberately leaked so that borrowers are not left with a dangling
+    // pointer.
+    void add_borrower();
+    void remove_borrower();
+
+    // Name of the owning bvar, purely for diagnostics.
+    void set_debug_name(const std::string& name) {
+        BAIDU_SCOPED_LOCK(_mutex);
+        _debug_name = name;
+    }
+    std::string debug_name() const {
+        BAIDU_SCOPED_LOCK(_mutex);
+        return _debug_name;
+    }
+
 protected:
     virtual ~Sampler();
 
 friend class SamplerCollector;
     bool _used;
-    // Sync destroy() and take_sample().
-    butil::Mutex _mutex;
+    // Number of external borrowers, guarded by _mutex.
+    int _nborrow;
+    // Set by destroy() when _nborrow > 0, telling the sampling thread to leak
+    // this sampler instead of deleting it. Guarded by _mutex.
+    bool _leaked;
+    mutable butil::Mutex _mutex;
+    // For diagnostics only, see set_debug_name().
+    std::string _debug_name;
 };
 
 // Representing a non-existing operator so that we can test
@@ -78,19 +108,90 @@ struct VoidOp {
     }
 };
 
+// Detects whether the host R exposes share_combiner(), namely whether its
+// sampling data lives in a shared_ptr-managed carrier (an AgentCombiner) that
+// the sampler is able to hold on its own. Hosts keeping the data elsewhere --
+// a user callback in PassiveStatus, or a value-type babylon counter -- do NOT
+// provide it and are sampled through the host pointer as before.
+template <typename R>
+class HasShareCombiner {
+    template <typename U>
+    static auto probe(U* p) -> decltype(p->share_combiner(), std::true_type());
+    static std::false_type probe(...);
+public:
+    static const bool value = decltype(probe(std::declval<R*>()))::value;
+};
+
+// Samples through the host pointer, for hosts keeping their data outside a
+// shared carrier (a user callback in PassiveStatus, a value-type babylon
+// counter, ...).
+template <typename R, typename T, typename Op, typename InvOp>
+class HostSampleSource {
+public:
+    explicit HostSampleSource(R* host)
+        : _host(host), _op(host->op()), _inv_op(host->inv_op()) {}
+
+    // Only reached from take_sample(), namely from the sampling thread, which is
+    // mutually exclusive with the host's destroy(). The host is therefore always
+    // alive here.
+    T reset() { return _host->reset(); }
+    T get_value() const { return _host->get_value(); }
+
+    // Never touch the host, see the ctor.
+    const Op& op() const { return _op; }
+    const InvOp& inv_op() const { return _inv_op; }
+
+private:
+    R* _host;
+    Op _op;
+    InvOp _inv_op;
+};
+
+// Samples directly from the shared data carrier, so that sampling still reads
+// valid memory even if the host is destructed before the sampler is recycled.
+// `Op'/`InvOp' are stateless functors, thus copied by value at construction and
+// the host is never touched afterwards.
+template <typename R, typename T, typename Op, typename InvOp>
+class CombinerSampleSource {
+public:
+    explicit CombinerSampleSource(R* host)
+        : _combiner(host->share_combiner())
+        , _op(host->op())
+        , _inv_op(host->inv_op()) {}
+
+    T reset() { return _combiner->reset_all_agents(); }
+    T get_value() const { return _combiner->combine_agents(); }
+    const Op& op() const { return _op; }
+    const InvOp& inv_op() const { return _inv_op; }
+
+private:
+    typename R::shared_combiner_type _combiner;
+    Op _op;
+    InvOp _inv_op;
+};
+
 // The sampler for reducer-alike variables.
 // The R should have following methods:
 //  - T reset();
 //  - T get_value();
 //  - Op op();
 //  - InvOp inv_op();
+// Additionally, if R exposes
+//  - shared_combiner_type share_combiner();
+// the sampler holds that shared carrier instead of R itself, which makes
+// sampling immune to R being destructed first.
 template <typename R, typename T, typename Op, typename InvOp>
 class ReducerSampler : public Sampler {
+    typedef typename butil::conditional<
+        HasShareCombiner<R>::value,
+        CombinerSampleSource<R, T, Op, InvOp>,
+        HostSampleSource<R, T, Op, InvOp> >::type source_type;
+
 public:
     static const time_t MAX_SECONDS_LIMIT = 3600;
 
     explicit ReducerSampler(R* reducer)
-        : _reducer(reducer)
+        : _source(reducer)
         , _window_size(1) {
 
         // Invoked take_sample at begining so the value of the first second
@@ -127,14 +228,14 @@ class ReducerSampler : public Sampler {
             // Suming up samples gives the result within a window.
             // In this case, get_value() of _reducer gives wrong answer and
             // should not be called.
-            latest.data = _reducer->reset();
+            latest.data = _source.reset();
         } else {
             // The operator can be inversed.
             // We save the result as a sample.
             // Inversed operation between latest and oldest sample within a
             // window gives result.
             // get_value() of _reducer can still be called.
-            latest.data = _reducer->get_value();
+            latest.data = _source.get_value();
         }
         latest.time_us = butil::cpuwide_time_us();
         _q.elim_push(latest);
@@ -164,12 +265,12 @@ class ReducerSampler : public Sampler {
                 if (e == oldest) {
                     break;
                 }
-                _reducer->op()(result->data, e->data);
+                _source.op()(result->data, e->data);
             }
         } else {
             // Diff the latest and oldest sample within the window.
             result->data = latest->data;
-            _reducer->inv_op()(result->data, oldest->data);
+            _source.inv_op()(result->data, oldest->data);
         }
         result->time_us = latest->time_us - oldest->time_us;
         return true;
@@ -212,7 +313,7 @@ class ReducerSampler : public Sampler {
     }
 
 private:
-    R* _reducer;
+    source_type _source;
     time_t _window_size;
     butil::BoundedQueue<Sample<T> > _q;
 };

diff --git a/src/bvar/latency_recorder.cpp b/src/bvar/latency_recorder.cpp
@@ -24,6 +24,25 @@
 
 namespace bvar {
 
+#if !WITH_BABYLON_COUNTER
+// Verify how ReducerSampler picks its data source, using the very hosts that
+// LatencyRecorder is made of.
+// Hosts keeping their data in a shared combiner are sampled through that carrier,
+// so sampling reads valid memory even if the host is destructed before the sampler
+// is recycled.
+// PassiveStatus keeps its data in a user callback instead, hence it is still sampled
+// through the host pointer.
+static_assert(detail::HasShareCombiner<IntRecorder>::value,
+              "IntRecorder should be sampled through its shared combiner");
+static_assert(detail::HasShareCombiner<Maxer<int64_t>::Base>::value,
+              "Reducer should be sampled through its shared combiner");
+static_assert(detail::HasShareCombiner<detail::Percentile>::value,
+              "Percentile should be sampled through its shared combiner");
+static_assert(!detail::HasShareCombiner<PassiveStatus<int64_t> >::value,
+              "PassiveStatus has no shared carrier, it must keep being sampled"
+              " through the host pointer");
+#endif // !WITH_BABYLON_COUNTER
+
 static bool valid_percentile(const char*, int32_t v) {
     return v > 0 && v < 100;
 }

diff --git a/src/bvar/passive_status.h b/src/bvar/passive_status.h
@@ -143,6 +143,7 @@ class PassiveStatus : public Variable {
     sampler_type* get_sampler() {
         if (nullptr == _sampler) {
             _sampler = new sampler_type(this);
+            _sampler->set_debug_name(name());
             _sampler->schedule();
         }
         return _sampler;
@@ -168,11 +169,16 @@ class PassiveStatus : public Variable {
 
 protected:
     int expose_impl(const butil::StringPiece& prefix,
-                    const butil::StringPiece& name,
+                    const butil::StringPiece& n,
                     DisplayFilter display_filter) override {
-        const int rc = Variable::expose_impl(prefix, name, display_filter);
+        const int rc = Variable::expose_impl(prefix, n, display_filter);
+        if (rc != 0) {
+            return rc;
+        }
+        if (_sampler != nullptr) {
+            _sampler->set_debug_name(name());
+        }
         if (ADDITIVE &&
-            rc == 0 &&
             _series_sampler == nullptr &&
             FLAGS_save_series) {
             _series_sampler = new SeriesSampler(this);