fuzz: Resolve h1/h2 codec fuzz flakes by using cleanup hooks rather t…

…han static pods (#29522)

 #10281 introduced persistent fuzz state to avoid rebuilding complex test infrastructure between test methods, and improve fuzzing performance. However it introduced a static-init fiasco by statically initiailizing a non-pod, which has non-deterministic destruction order compared to other static-inits.

This causes flaky tests, particularly on ARM.

This PR adds a new mechanism to the fuzz-runner infrastructure to allow cleanup hooks to be established, that will be run after all test methods but before main() returns, in a deterministic, platform-independent order.

Additional Description: n/a
Risk Level: low -- test only
Testing: //test/integration/...

Signed-off-by: Joshua Marantz <jmarantz@google.com>

test/fuzz/fuzz_runner.cc

@@ -56,6 +56,27 @@ void Runner::setupEnvironment(int argc, char** argv, spdlog::level::level_enum d
   }
 }
 
+using Hooks = std::vector<std::function<void()>>;
+static Hooks* cleanup_hooks = nullptr;
+
+void addCleanupHook(std::function<void()> cleanup) {
+  if (cleanup_hooks == nullptr) {
+    cleanup_hooks = new Hooks;
+  }
+  cleanup_hooks->push_back(cleanup);
+}
+
+void runCleanupHooks() {
+  if (cleanup_hooks != nullptr) {
+    // Run hooks in reverse order from how they were added.
+    for (auto iter = cleanup_hooks->rbegin(), end = cleanup_hooks->rend(); iter != end; ++iter) {
+      (*iter)();
+    }
+    delete cleanup_hooks;
+    cleanup_hooks = nullptr;
+  }
+}
+
 } // namespace Fuzz
 } // namespace Envoy
 

test/fuzz/fuzz_runner.h

@@ -52,6 +52,18 @@ class Runner {
   static spdlog::level::level_enum log_level_;
 };
 
+/**
+ * Establishes a function to run before the test process exits. This enables
+ * threads, mocks, and other objects that are expensive to create to be shared
+ * between test methods.
+ */
+void addCleanupHook(std::function<void()>);
+
+/**
+ * Runs all cleanup hooks.
+ */
+void runCleanupHooks();
+
 } // namespace Fuzz
 } // namespace Envoy
 
@@ -63,9 +75,13 @@ extern "C" int LLVMFuzzerInitialize(int* argc, char*** argv);
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size);
 
 #ifdef PERSISTENT_FUZZER
-#define PERSISTENT_FUZZ_VAR static
+template <typename T> T& initFuzzVar(T* ptr) {
+  Envoy::Fuzz::addCleanupHook([ptr]() { delete ptr; });
+  return *ptr;
+}
+#define PERSISTENT_FUZZ_VAR(type, var, args) static type& var = initFuzzVar(new type(args))
 #else
-#define PERSISTENT_FUZZ_VAR
+#define PERSISTENT_FUZZ_VAR(type, var, args) type var args
 #endif
 
 #define DEFINE_TEST_ONE_INPUT_IMPL                                                                 \

test/fuzz/main.cc

@@ -94,5 +94,7 @@ int main(int argc, char** argv) {
   testing::InitGoogleMock(&argc, argv);
   Envoy::Fuzz::Runner::setupEnvironment(argc, argv, spdlog::level::info);
 
-  return RUN_ALL_TESTS();
+  int status = RUN_ALL_TESTS();
+  Envoy::Fuzz::runCleanupHooks();
+  return status;
 }

test/integration/h1_capture_direct_response_fuzz_test.cc

@@ -32,7 +32,7 @@ void H1FuzzIntegrationTest::initialize() {
 DEFINE_PROTO_FUZZER(const test::integration::CaptureFuzzTestCase& input) {
   RELEASE_ASSERT(!TestEnvironment::getIpVersionsForTest().empty(), "");
   const auto ip_version = TestEnvironment::getIpVersionsForTest()[0];
-  PERSISTENT_FUZZ_VAR H1FuzzIntegrationTest h1_fuzz_integration_test(ip_version);
+  PERSISTENT_FUZZ_VAR(H1FuzzIntegrationTest, h1_fuzz_integration_test, (ip_version));
   h1_fuzz_integration_test.replay(input, true);
 }
 

test/integration/h1_capture_fuzz_test.cc

@@ -7,7 +7,7 @@ DEFINE_PROTO_FUZZER(const test::integration::CaptureFuzzTestCase& input) {
   // Pick an IP version to use for loopback, it doesn't matter which.
   RELEASE_ASSERT(!TestEnvironment::getIpVersionsForTest().empty(), "");
   const auto ip_version = TestEnvironment::getIpVersionsForTest()[0];
-  PERSISTENT_FUZZ_VAR H1FuzzIntegrationTest h1_fuzz_integration_test(ip_version);
+  PERSISTENT_FUZZ_VAR(H1FuzzIntegrationTest, h1_fuzz_integration_test, (ip_version));
   h1_fuzz_integration_test.replay(input, false);
 }
 

test/integration/h1_fuzz.cc

@@ -12,10 +12,10 @@ namespace Envoy {
 
 void H1FuzzIntegrationTest::replay(const test::integration::CaptureFuzzTestCase& input,
                                    bool ignore_response) {
-  PERSISTENT_FUZZ_VAR bool initialized = [this]() -> bool {
-    initialize();
-    return true;
-  }();
+  struct Init {
+    Init(H1FuzzIntegrationTest* test) { test->initialize(); }
+  };
+  PERSISTENT_FUZZ_VAR(Init, initialized, (this));
   UNREFERENCED_PARAMETER(initialized);
   IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("http"));
   FakeRawConnectionPtr fake_upstream_connection;

test/integration/h2_capture_direct_response_fuzz_test.cc

@@ -36,7 +36,7 @@ void H2FuzzIntegrationTest::initialize() {
 DEFINE_PROTO_FUZZER(const test::integration::H2CaptureFuzzTestCase& input) {
   RELEASE_ASSERT(!TestEnvironment::getIpVersionsForTest().empty(), "");
   const auto ip_version = TestEnvironment::getIpVersionsForTest()[0];
-  PERSISTENT_FUZZ_VAR H2FuzzIntegrationTest h2_fuzz_integration_test(ip_version);
+  PERSISTENT_FUZZ_VAR(H2FuzzIntegrationTest, h2_fuzz_integration_test, (ip_version));
   h2_fuzz_integration_test.replay(input, true);
 }
 

test/integration/h2_capture_fuzz_test.cc

@@ -24,7 +24,7 @@ DEFINE_PROTO_FUZZER(const test::integration::H2CaptureFuzzTestCase& input) {
   // Pick an IP version to use for loopback, it doesn't matter which.
   FUZZ_ASSERT(!TestEnvironment::getIpVersionsForTest().empty());
   const auto ip_version = TestEnvironment::getIpVersionsForTest()[0];
-  PERSISTENT_FUZZ_VAR H2FuzzIntegrationTest h2_fuzz_integration_test(ip_version);
+  PERSISTENT_FUZZ_VAR(H2FuzzIntegrationTest, h2_fuzz_integration_test, (ip_version));
   h2_fuzz_integration_test.replay(input, false);
 }
 

test/integration/h2_fuzz.cc

@@ -169,10 +169,10 @@ void H2FuzzIntegrationTest::sendFrame(const test::integration::H2TestFrame& prot
 
 void H2FuzzIntegrationTest::replay(const test::integration::H2CaptureFuzzTestCase& input,
                                    bool ignore_response) {
-  PERSISTENT_FUZZ_VAR bool initialized = [this]() -> bool {
-    initialize();
-    return true;
-  }();
+  struct Init {
+    Init(H2FuzzIntegrationTest* test) { test->initialize(); }
+  };
+  PERSISTENT_FUZZ_VAR(Init, initialized, (this));
   UNREFERENCED_PARAMETER(initialized);
   IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("http"));
   FakeRawConnectionPtr fake_upstream_connection;