From 234e2ed5b69a45cfd180d272e47a3bb9481bcb7f Mon Sep 17 00:00:00 2001
From: anand76 <anand76@devvm1373.frc2.facebook.com>
Date: Tue, 14 Apr 2020 11:04:39 -0700
Subject: [PATCH] Fix a couple of bugs in db_stress fault injection (#6700)

Summary:
1. Fix a memory leak in FaultInjectionTestFS in the stack trace related
code
2. Check status of all MultiGet keys before deciding whether an error
was swallowed, instead of assuming an ok status for any key means an
undetected error
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6700

Test Plan: Run db_stress with asan and fault injection

Reviewed By: cheng-chang

Differential Revision: D21021498

Pulled By: anand1976

fbshipit-source-id: 489191efd1ab0fa834923a1e1d57253a7a315465
---
 db_stress_tool/no_batched_ops_stress.cc | 39 +++++++++++++++----------
 test_util/fault_injection_test_fs.cc    |  4 +++
 test_util/fault_injection_test_fs.h     |  6 ++++
 3 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/db_stress_tool/no_batched_ops_stress.cc b/db_stress_tool/no_batched_ops_stress.cc
index ed5586162..4fc8872bf 100644
--- a/db_stress_tool/no_batched_ops_stress.cc
+++ b/db_stress_tool/no_batched_ops_stress.cc
@@ -169,7 +169,7 @@ class NonBatchedOpsStressTest : public StressTest {
           // stack trace at the same time
           MutexLock l(thread->shared->GetMutex());
           fprintf(stderr, "Didn't get expected error from Get\n");
-          fprintf(stderr, "Callstack that injected the error\n");
+          fprintf(stderr, "Callstack that injected the fault\n");
           fault_fs_guard->PrintFaultBacktrace();
           std::terminate();
         }
@@ -290,24 +290,31 @@ class NonBatchedOpsStressTest : public StressTest {
 #endif
     }
 
-    for (const auto& s : statuses) {
-      if (s.ok()) {
-#ifndef NDEBUG
-        if (fault_fs_guard && error_count && !SharedState::filter_read_error) {
-          // Grab mutex so multiple thread don't try to print the
-          // stack trace at the same time
-          MutexLock l(thread->shared->GetMutex());
-          fprintf(stderr, "Didn't get expected error from MultiGet\n");
-          fprintf(stderr, "Callstack that injected the error\n");
-          fault_fs_guard->PrintFaultBacktrace();
-          std::terminate();
-        } else {
-#endif // NDEBUG
-          // found case
-          thread->stats.AddGets(1, 1);
 #ifndef NDEBUG
+    if (fault_fs_guard && error_count && !SharedState::filter_read_error) {
+      int stat_nok = 0;
+      for (const auto& s : statuses) {
+        if (!s.ok() && !s.IsNotFound()) {
+          stat_nok++;
         }
+      }
+
+      if (stat_nok < error_count) {
+        // Grab mutex so multiple thread don't try to print the
+        // stack trace at the same time
+        MutexLock l(thread->shared->GetMutex());
+        fprintf(stderr, "Didn't get expected error from MultiGet\n");
+        fprintf(stderr, "Callstack that injected the fault\n");
+        fault_fs_guard->PrintFaultBacktrace();
+        std::terminate();
+      }
+    }
 #endif // NDEBUG
+
+    for (const auto& s : statuses) {
+      if (s.ok()) {
+        // found case
+        thread->stats.AddGets(1, 1);
       } else if (s.IsNotFound()) {
         // not found case
         thread->stats.AddGets(1, 0);
diff --git a/test_util/fault_injection_test_fs.cc b/test_util/fault_injection_test_fs.cc
index 9aff21cd7..2658b8e37 100644
--- a/test_util/fault_injection_test_fs.cc
+++ b/test_util/fault_injection_test_fs.cc
@@ -473,6 +473,9 @@ IOStatus FaultInjectionTestFS::InjectError(ErrorOperation op,
 
   if (ctx->rand.OneIn(ctx->one_in)) {
     ctx->count++;
+    if (ctx->callstack) {
+      free(ctx->callstack);
+    }
     ctx->callstack = port::SaveStack(&ctx->frames);
     switch (op) {
       case kRead:
@@ -530,6 +533,7 @@ void FaultInjectionTestFS::PrintFaultBacktrace() {
     return;
   }
   port::PrintAndFreeStack(ctx->callstack, ctx->frames);
+  ctx->callstack = nullptr;
 #endif
 }
 
diff --git a/test_util/fault_injection_test_fs.h b/test_util/fault_injection_test_fs.h
index 4c8c31a28..adebc21a2 100644
--- a/test_util/fault_injection_test_fs.h
+++ b/test_util/fault_injection_test_fs.h
@@ -366,7 +366,13 @@ class FaultInjectionTestFS : public FileSystemWrapper {
     explicit ErrorContext(uint32_t seed)
       : rand(seed),
         enable_error_injection(false),
+        callstack(nullptr),
     frames(0) {}
+    ~ErrorContext() {
+      if (callstack) {
+        free(callstack);
+      }
+    }
   };
 
   std::unique_ptr<ThreadLocalPtr> thread_local_error_;