FaultInjection: Fix random number generation

BUILD

@@ -3843,7 +3843,9 @@ grpc_cc_library(
         "src/core/ext/filters/fault_injection/service_config_parser.h",
     ],
     external_deps = [
+        "absl/base:core_headers",
         "absl/memory",
+        "absl/random",
         "absl/status",
         "absl/status:statusor",
         "absl/strings",

src/core/ext/filters/fault_injection/fault_injection_filter.cc

@@ -19,7 +19,6 @@
 #include "src/core/ext/filters/fault_injection/fault_injection_filter.h"
 
 #include <stdint.h>
-#include <stdlib.h>
 
 #include <algorithm>
 #include <atomic>
@@ -70,12 +69,14 @@ auto AsInt(absl::string_view s) -> absl::optional<T> {
   return absl::nullopt;
 }
 
-inline bool UnderFraction(const uint32_t numerator,
+inline bool UnderFraction(absl::InsecureBitGen* rand_generator,
+                          const uint32_t numerator,
                           const uint32_t denominator) {
   if (numerator <= 0) return false;
   if (numerator >= denominator) return true;
   // Generate a random number in [0, denominator).
-  const uint32_t random_number = rand() % denominator;
+  const uint32_t random_number =
+      absl::Uniform(absl::IntervalClosedOpen, *rand_generator, 0u, denominator);
   return random_number < numerator;
 }
 
@@ -139,7 +140,8 @@ FaultInjectionFilter::FaultInjectionFilter(ChannelFilter::Args filter_args)
           filter_args.channel_stack(),
           filter_args.uninitialized_channel_element())),
       service_config_parser_index_(
-          FaultInjectionServiceConfigParser::ParserIndex()) {}
+          FaultInjectionServiceConfigParser::ParserIndex()),
+      mu_(new Mutex) {}
 
 // Construct a promise for one call.
 ArenaPromise<ServerMetadataHandle> FaultInjectionFilter::MakeCallPromise(
@@ -219,14 +221,21 @@ FaultInjectionFilter::MakeInjectionDecision(
     }
   }
   // Roll the dice
-  const bool delay_request =
-      delay != Duration::Zero() &&
-      UnderFraction(delay_percentage_numerator,
-                    fi_policy->delay_percentage_denominator);
-  const bool abort_request =
-      abort_code != GRPC_STATUS_OK &&
-      UnderFraction(abort_percentage_numerator,
-                    fi_policy->abort_percentage_denominator);
+  bool delay_request = delay != Duration::Zero();
+  bool abort_request = abort_code != GRPC_STATUS_OK;
+  if (delay_request || abort_request) {
+    MutexLock lock(mu_.get());
+    if (delay_request) {
+      delay_request =
+          UnderFraction(&delay_rand_generator_, delay_percentage_numerator,
+                        fi_policy->delay_percentage_denominator);
+    }
+    if (abort_request) {
+      abort_request =
+          UnderFraction(&abort_rand_generator_, abort_percentage_numerator,
+                        fi_policy->abort_percentage_denominator);
+    }
+  }
 
   return InjectionDecision(
       fi_policy->max_faults, delay_request ? delay : Duration::Zero(),

src/core/ext/filters/fault_injection/fault_injection_filter.h

@@ -21,11 +21,16 @@
 
 #include <stddef.h>
 
+#include <memory>
+
+#include "absl/base/thread_annotations.h"
+#include "absl/random/random.h"
 #include "absl/status/statusor.h"
 
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/channel/channel_fwd.h"
 #include "src/core/lib/channel/promise_based_filter.h"
+#include "src/core/lib/gprpp/sync.h"
 #include "src/core/lib/promise/arena_promise.h"
 #include "src/core/lib/transport/transport.h"
 
@@ -60,6 +65,9 @@ class FaultInjectionFilter : public ChannelFilter {
   // The relative index of instances of the same filter.
   size_t index_;
   const size_t service_config_parser_index_;
+  std::unique_ptr<Mutex> mu_;
+  absl::InsecureBitGen abort_rand_generator_ ABSL_GUARDED_BY(mu_);
+  absl::InsecureBitGen delay_rand_generator_ ABSL_GUARDED_BY(mu_);
 };
 
 }  // namespace grpc_core

test/cpp/end2end/xds/xds_fault_injection_end2end_test.cc

@@ -204,7 +204,13 @@ TEST_P(FaultInjectionTest, XdsFaultInjectionPercentageAbortViaHeaders) {
 
 TEST_P(FaultInjectionTest, XdsFaultInjectionPercentageDelay) {
   CreateAndStartBackends(1);
+  const uint32_t kFixedDelayMilliseconds = 100000;
+#ifndef NDEBUG
   const uint32_t kRpcTimeoutMilliseconds = grpc_test_slowdown_factor() * 3000;
+#else
+  const uint32_t kRpcTimeoutMilliseconds = grpc_test_slowdown_factor() * 6000;
+#endif
+  const uint32_t kDelayPercentageCap = 100;
   const uint32_t kFixedDelaySeconds = 100;
   const uint32_t kDelayPercentagePerHundred = 50;
   const double kDelayRate = kDelayPercentagePerHundred / 100.0;
@@ -250,7 +256,11 @@ TEST_P(FaultInjectionTest, XdsFaultInjectionPercentageDelay) {
 TEST_P(FaultInjectionTest, XdsFaultInjectionPercentageDelayViaHeaders) {
   CreateAndStartBackends(1);
   const uint32_t kFixedDelayMilliseconds = 100000;
+#ifndef NDEBUG
   const uint32_t kRpcTimeoutMilliseconds = grpc_test_slowdown_factor() * 3000;
+#else
+  const uint32_t kRpcTimeoutMilliseconds = grpc_test_slowdown_factor() * 6000;
+#endif const uint32_t kDelayPercentageCap = 100;
   const uint32_t kDelayPercentageCap = 100;
   const uint32_t kDelayPercentage = 50;
   const double kDelayRate = kDelayPercentage / 100.0;

tools/distrib/fix_build_deps.py

@@ -85,6 +85,8 @@
         'absl/meta:type_traits',
     'absl/random/random.h':
         'absl/random',
+    'absl/random/distributions.h':
+        'absl/random:distributions',
     'absl/status/status.h':
         'absl/status',
     'absl/status/statusor.h':