Add metric to cacth the double free error in mmap allocator (facebook…

…incubator#8648)

Summary:
Currently we only have a warning log for double free in mmap allocator
and we shall also add a metric for production alert.

Pull Request resolved: facebookincubator#8648

Reviewed By: bikramSingh91

Differential Revision: D53341067

Pulled By: xiaoxmeng

fbshipit-source-id: 3d0979a1621ebae1180b3d59cd98345fd0c4dff8

velox/common/base/Counters.cpp

@@ -146,7 +146,6 @@ void registerVeloxMetrics() {
   // The distribution of a root memory pool's initial capacity in range of [0,
   // 256MB] with 32 buckets. It is configured to report the capacity at P50,
   // P90, P99, and P100 percentiles.
-
   DEFINE_HISTOGRAM_METRIC(
       kMetricMemoryPoolInitialCapacityBytes,
       8L << 20,
@@ -163,6 +162,12 @@ void registerVeloxMetrics() {
   DEFINE_HISTOGRAM_METRIC(
       kMetricMemoryPoolCapacityGrowCount, 8, 0, 256, 50, 90, 99, 100);
 
+  // Tracks the count of double frees in memory allocator, indicating the
+  // possibility of buffer ownership issues when a buffer is freed more than
+  // once.
+  DEFINE_METRIC(
+      kMetricMemoryAllocatorDoubleFreeCount, facebook::velox::StatType::COUNT);
+
   /// ================== Spill related Counters =================
 
   // The number of bytes in memory to spill.

velox/common/base/Counters.h

@@ -67,6 +67,9 @@ constexpr folly::StringPiece kMetricMemoryPoolUsageLeakBytes{
 constexpr folly::StringPiece kMetricMemoryPoolReservationLeakBytes{
     "velox.memory_pool_reservation_leak_bytes"};
 
+constexpr folly::StringPiece kMetricMemoryAllocatorDoubleFreeCount{
+    "velox.memory_allocator_double_free_count"};
+
 constexpr folly::StringPiece kMetricArbitratorRequestsCount{
     "velox.arbitrator_requests_count"};
 

velox/common/memory/MmapAllocator.cpp

@@ -18,7 +18,9 @@
 
 #include <sys/mman.h>
 
+#include "velox/common/base/Counters.h"
 #include "velox/common/base/Portability.h"
+#include "velox/common/base/StatsReporter.h"
 #include "velox/common/memory/Memory.h"
 
 namespace facebook::velox::memory {
@@ -877,10 +879,10 @@ MachinePageCount MmapAllocator::SizeClass::free(Allocation& allocation) {
     const int firstBit =
         (runAddress - address_) / (AllocationTraits::kPageSize * unitSize_);
     for (auto page = firstBit; page < firstBit + numPages; ++page) {
-      if (!bits::isBitSet(pageAllocated_.data(), page)) {
-        // TODO: change this to a velox failure to catch the bug.
+      if (FOLLY_UNLIKELY(!bits::isBitSet(pageAllocated_.data(), page))) {
         VELOX_MEM_LOG(ERROR)
             << "Double free: page = " << page << " sizeclass = " << unitSize_;
+        RECORD_METRIC_VALUE(kMetricMemoryAllocatorDoubleFreeCount);
         continue;
       }
       if (bits::isBitSet(pageMapped_.data(), page)) {

velox/docs/monitoring/metrics.rst

@@ -175,6 +175,11 @@ Memory Management
    * - memory_pool_capacity_leak_bytes
      - Sum
      - The root memory pool reservation leak in bytes.
+   * - memory_allocator_double_free_count
+     - Count
+     - Tracks the count of double frees in memory allocator, indicating the
+       possibility of buffer ownership issues when a buffer is freed more
+       than once.
 
 Spilling
 --------