enable explicit clean-up prior to process exit (#2782)

Co-authored-by: Dhiren Vispute <[email protected]>

.github/workflows/cicd.yml

@@ -367,6 +367,21 @@ jobs:
       fault_injection: true
       leak_detection: true
 
+  # Run a fast multi-threaded stress test pass against the usersim user-mode 'mock' framework.
+  # Added as a 'per-PR' test to catch usersim regressions and/or run-time usage issues.
+  quick_user_mode_multi_threaded_stress_test:
+    needs: regular
+    uses: ./.github/workflows/reusable-test.yml
+    with:
+      name: quick_user_mode_multi_threaded_stress
+      test_command: .\ebpf_stress_tests_um -tt=8 -td=2
+      build_artifact: Build-x64
+      environment: windows-2022
+      code_coverage: false
+      leak_detection: false
+      gather_dumps: true
+      capture_etw: true
+
   # Additional jobs to run on a schedule only (skip push and pull request).
   # ---------------------------------------------------------------------------
   codeql:
@@ -416,7 +431,7 @@ jobs:
     uses: ./.github/workflows/reusable-test.yml
     with:
       name: user_mode_multi_threaded_stress
-      test_command: .\ebpf_stress_tests_um -tt=32 -td=10
+      test_command: .\ebpf_stress_tests_um -tt=8 -td=10
       build_artifact: Build-x64
       environment: windows-2022
       code_coverage: false

tests/end_to_end/helpers.h

@@ -115,7 +115,7 @@ typedef class _single_instance_hook : public _hook_helper
     _single_instance_hook(ebpf_program_type_t program_type, ebpf_attach_type_t attach_type)
         : _hook_helper{attach_type}, client_binding_context(nullptr), client_data(nullptr),
           client_dispatch_table(nullptr), link_object(nullptr), client_registration_instance(nullptr),
-          nmr_binding_handle(nullptr)
+          nmr_binding_handle(nullptr), nmr_provider_handle(nullptr)
     {
         attach_provider_data.supported_program_type = program_type;
         attach_provider_data.bpf_attach_type = get_bpf_attach_type(&attach_type);

tests/stress/ebpf_mt_stress.h

@@ -104,3 +104,7 @@ get_jit_program_attributes(const std::string& program_name);
 // The query_supported_program_names() call is 'exported' by both the user and kernel mode test suites.
 const std::vector<std::string>
 query_supported_program_names();
+
+// The test_process_cleanup() call is 'exported' by both the user and kernel mode test suites.
+void
+test_process_cleanup();

tests/stress/ebpf_stress_tests.cpp

@@ -153,4 +153,9 @@ main(int argc, char* argv[])
     }
 
     session.run();
+
+    // Tests that run against the 'usersim' framework need explicit clean-up before process
+    // termination. This clean-up is handled by the OS and/or the in-kernel eBPF components for
+    // tests that run against the kernel components.
+    test_process_cleanup();
 }

tests/stress/km/stress_tests_km.cpp

@@ -44,6 +44,14 @@ query_supported_program_names()
     return program_names;
 }
 
+// This function is called by the common test initialization code to perform the requisite clean-up as the last action
+// prior to process termination.
+void
+test_process_cleanup()
+{
+    // As of now, we don't need to do anything here for kernel mode tests.
+}
+
 // Test thread control parameters (# of threads, run duration etc.).
 static test_control_info _global_test_control_info{0};
 
@@ -1545,4 +1553,4 @@ TEST_CASE("bindmonitor_tail_call_invoke_program_test", "[native_mt_stress_test]"
 
     _print_test_control_info(local_test_control_info);
     _mt_bindmonitor_tail_call_invoke_program_test(local_test_control_info);
-}
+}

tests/stress/um/stress_tests_um.cpp

@@ -218,29 +218,33 @@ query_supported_program_names()
 // These objects should be created just _once_ per (test) process. This is only needed for user mode tests that use the
 // user mode 'mock' framework.  Note that these cannot be created globally and _must_ be created in the context of a
 // a Catch2 test 'session' (Per Catch2 documentation, Catch2's exception framework is apparently not quite ready until
-// then).  This is an issue in our usage as we make extensive use of Catch2's REQUIRE verification/validation macros
-// (based on this framework) during the creation of these objects;
-static _test_helper_end_to_end* _test_helper{nullptr};
-static program_info_provider_t* _bind_program_info_provider{nullptr};
-static program_info_provider_t* _xdp_program_info_provider{nullptr};
+// then).  This becomes an issue in our usage as we make extensive use of Catch2's REQUIRE verification/validation
+// macros (based on this framework) during the creation of these objects;
+static std::unique_ptr<_test_helper_end_to_end> _test_helper;
+static std::unique_ptr<program_info_provider_t> _bind_program_info_provider;
+static std::unique_ptr<program_info_provider_t> _xdp_program_info_provider;
+
 static test_control_info _test_control_info{0};
 
 static std::once_flag _um_test_init_done;
 static void
 um_test_init()
 {
     std::call_once(_um_test_init_done, [&]() {
-        _test_helper = new _test_helper_end_to_end;
-        REQUIRE(_test_helper != nullptr);
-        _test_helper->initialize();
+        _test_helper_end_to_end* local_test_helper = new _test_helper_end_to_end;
+        REQUIRE(local_test_helper != nullptr);
+        local_test_helper->initialize();
+        _test_helper.reset(local_test_helper);
 
-        _bind_program_info_provider = new program_info_provider_t();
-        REQUIRE(_bind_program_info_provider->initialize(EBPF_PROGRAM_TYPE_BIND) == EBPF_SUCCESS);
-        REQUIRE(_bind_program_info_provider != nullptr);
+        program_info_provider_t* local_bind_program_info_provider = new program_info_provider_t();
+        REQUIRE(local_bind_program_info_provider != nullptr);
+        REQUIRE(local_bind_program_info_provider->initialize(EBPF_PROGRAM_TYPE_BIND) == EBPF_SUCCESS);
+        _bind_program_info_provider.reset(local_bind_program_info_provider);
 
-        _xdp_program_info_provider = new program_info_provider_t();
-        REQUIRE(_xdp_program_info_provider->initialize(EBPF_PROGRAM_TYPE_XDP) == EBPF_SUCCESS);
-        REQUIRE(_xdp_program_info_provider != nullptr);
+        program_info_provider_t* local_xdp_program_info_provider = new program_info_provider_t();
+        REQUIRE(local_xdp_program_info_provider != nullptr);
+        REQUIRE(local_xdp_program_info_provider->initialize(EBPF_PROGRAM_TYPE_XDP) == EBPF_SUCCESS);
+        _xdp_program_info_provider.reset(local_xdp_program_info_provider);
 
         _test_control_info = get_test_control_info();
         if (_test_control_info.programs.size()) {
@@ -269,6 +273,17 @@ um_test_init()
     });
 }
 
+// This function is called by the common test initialization code to perform the requisite clean-up as the last action
+// prior to process termination.
+void
+test_process_cleanup()
+{
+    // We need to explicitly 'free' these resources in tests that run against the user-mode 'usersim' framework.
+    _xdp_program_info_provider.reset(nullptr);
+    _bind_program_info_provider.reset(nullptr);
+    _test_helper.reset(nullptr);
+}
+
 TEST_CASE("load_attach_detach_unload_sequential_test", "[mt_stress_test]")
 {
     um_test_init();