From eaf3afec85af54546e194120449c8fb617e374bc Mon Sep 17 00:00:00 2001
From: xiaoxmeng <xiaoxmeng@fb.com>
Date: Thu, 1 Feb 2024 22:08:36 -0800
Subject: [PATCH] Add metric to cacth the double free error in mmap allocator

Currently we only have a warning log for double free in mmap allocator
and we shall also add a metric for production alert.
---
 velox/common/base/Counters.cpp        | 4 ++++
 velox/common/base/Counters.h          | 3 +++
 velox/common/memory/MmapAllocator.cpp | 6 ++++--
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/velox/common/base/Counters.cpp b/velox/common/base/Counters.cpp
index 1389f689a0c9..880246e05701 100644
--- a/velox/common/base/Counters.cpp
+++ b/velox/common/base/Counters.cpp
@@ -143,6 +143,10 @@ void registerVeloxMetrics() {
   DEFINE_METRIC(
       kMetricMemoryPoolReservationLeakBytes, facebook::velox::StatType::SUM);
 
+  // Tracks the count of double frees in memory allocator.
+  DEFINE_METRIC(
+      kMetricMemoryAllocatorDoubleFreeCount, facebook::velox::StatType::COUNT);
+
   /// ================== Spill related Counters =================
 
   // The number of bytes in memory to spill.
diff --git a/velox/common/base/Counters.h b/velox/common/base/Counters.h
index 1ad847206331..17024c043bec 100644
--- a/velox/common/base/Counters.h
+++ b/velox/common/base/Counters.h
@@ -61,6 +61,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"};
 
diff --git a/velox/common/memory/MmapAllocator.cpp b/velox/common/memory/MmapAllocator.cpp
index 2383b33698ea..65d5abe7aa5d 100644
--- a/velox/common/memory/MmapAllocator.cpp
+++ b/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)) {