From f624134bb02da227edf7d86cceec3c34e1a0aa9d Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 3 May 2024 18:20:32 +0000 Subject: [PATCH 1/5] build --- .../event_engine/event_engine_test_utils.cc | 4 ++ test/core/test_util/build.cc | 70 ++++++++++++------- test/core/test_util/build.h | 3 + 3 files changed, 51 insertions(+), 26 deletions(-) diff --git a/test/core/event_engine/event_engine_test_utils.cc b/test/core/event_engine/event_engine_test_utils.cc index cc036ad5d3b..1de11122b19 100644 --- a/test/core/event_engine/event_engine_test_utils.cc +++ b/test/core/event_engine/event_engine_test_utils.cc @@ -41,6 +41,7 @@ #include "src/core/lib/gprpp/notification.h" #include "src/core/lib/gprpp/time.h" #include "src/core/lib/resource_quota/memory_quota.h" +#include "test/core/test_util/build.h" // IWYU pragma: no_include @@ -77,7 +78,10 @@ std::string GetNextSendMessage() { } void WaitForSingleOwner(std::shared_ptr engine) { + int n = 0; while (engine.use_count() > 1) { + ++n; + if (n == 100) AsanAssertNoLeaks(); GRPC_LOG_EVERY_N_SEC(2, GPR_INFO, "engine.use_count() = %ld", engine.use_count()); absl::SleepFor(absl::Milliseconds(100)); diff --git a/test/core/test_util/build.cc b/test/core/test_util/build.cc index 04d39e9574d..2ae07567d5b 100644 --- a/test/core/test_util/build.cc +++ b/test/core/test_util/build.cc @@ -12,62 +12,80 @@ // See the License for the specific language governing permissions and // limitations under the License. -bool BuiltUnderValgrind() { -#ifdef RUNNING_ON_VALGRIND - return true; -#else - return false; -#endif -} +#include "test/core/test_util/build.h" -bool BuiltUnderTsan() { +// Define GRPC_BUILD_HAS_ASAN as 1 or 0 depending on if we're building under +// ASAN. #if defined(__has_feature) -#if __has_feature(thread_sanitizer) - return true; +#if __has_feature(address_sanitizer) +#define GRPC_BUILD_HAS_ASAN 1 #else - return false; +#define GRPC_BUILD_HAS_ASAN 0 #endif #else -#ifdef THREAD_SANITIZER - return true; +#ifdef ADDRESS_SANITIZER +#define GRPC_BUILD_HAS_ASAN 1 #else - return false; +#define GRPC_BUILD_HAS_ASAN 0 #endif #endif -} -bool BuiltUnderAsan() { +// Define GRPC_BUILD_HAS_TSAN as 1 or 0 depending on if we're building under +// TSAN. #if defined(__has_feature) -#if __has_feature(address_sanitizer) - return true; +#if __has_feature(thread_sanitizer) +#define GRPC_BUILD_HAS_TSAN 1 #else - return false; +#define GRPC_BUILD_HAS_TSAN 0 #endif #else -#ifdef ADDRESS_SANITIZER - return true; +#ifdef THREAD_SANITIZER +#define GRPC_BUILD_HAS_TSAN 1 #else - return false; +#define GRPC_BUILD_HAS_TSAN 0 #endif #endif -} -bool BuiltUnderMsan() { +// Define GRPC_BUILD_HAS_MSAN as 1 or 0 depending on if we're building under +// MSAN. #if defined(__has_feature) #if __has_feature(memory_sanitizer) - return true; +#define GRPC_BUILD_HAS_MSAN 1 #else - return false; +#define GRPC_BUILD_HAS_MSAN 0 #endif #else #ifdef MEMORY_SANITIZER +#define GRPC_BUILD_HAS_MSAN 1 +#else +#define GRPC_BUILD_HAS_MSAN 0 +#endif +#endif + +#if GRPC_BUILD_HAS_ASAN +#include +#endif + +bool BuiltUnderValgrind() { +#ifdef RUNNING_ON_VALGRIND return true; #else return false; #endif +} + +bool BuiltUnderTsan() { return GRPC_BUILD_HAS_TSAN != 0; } + +bool BuiltUnderAsan() { return GRPC_BUILD_HAS_ASAN != 0; } + +void AsanAssertNoLeaks() { +#if GRPC_BUILD_HAS_ASAN + __lsan_do_leak_check(); #endif } +bool BuiltUnderMsan() { return GRPC_BUILD_HAS_MSAN != 0; } + bool BuiltUnderUbsan() { #ifdef GRPC_UBSAN return true; diff --git a/test/core/test_util/build.h b/test/core/test_util/build.h index 20bc21954ff..8574dfe8d3c 100644 --- a/test/core/test_util/build.h +++ b/test/core/test_util/build.h @@ -30,4 +30,7 @@ bool BuiltUnderMsan(); // Returns whether this is built under UndefinedBehaviorSanitizer bool BuiltUnderUbsan(); +// Force a leak check if built under ASAN. If there are leaks, crash. +void AsanAssertNoLeaks(); + #endif // GRPC_TEST_CORE_TEST_UTIL_BUILD_H From 6c72434ef380ebaee39544a556973665842e902a Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 3 May 2024 18:23:50 +0000 Subject: [PATCH 2/5] x --- test/core/event_engine/event_engine_test_utils.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/event_engine/event_engine_test_utils.cc b/test/core/event_engine/event_engine_test_utils.cc index 1de11122b19..32ab6ac5276 100644 --- a/test/core/event_engine/event_engine_test_utils.cc +++ b/test/core/event_engine/event_engine_test_utils.cc @@ -81,7 +81,7 @@ void WaitForSingleOwner(std::shared_ptr engine) { int n = 0; while (engine.use_count() > 1) { ++n; - if (n == 100) AsanAssertNoLeaks(); + if (n % 100 == 0) AsanAssertNoLeaks(); GRPC_LOG_EVERY_N_SEC(2, GPR_INFO, "engine.use_count() = %ld", engine.use_count()); absl::SleepFor(absl::Milliseconds(100)); From 87c42aab68c817e5a9240ce7fabf2cfc7362cd85 Mon Sep 17 00:00:00 2001 From: AJ Heller Date: Tue, 7 May 2024 11:43:06 -0700 Subject: [PATCH 3/5] [test] Wait for EE to quiesce in RLSEnd2endTest teardown (#36553) Closes #36553 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36553 from drfloob:cleaner-RlsEnd2endTest-shutdown f16b5aac3e5f73154a7944791de61dc050f05ec3 PiperOrigin-RevId: 631496415 --- CMakeLists.txt | 1 + build_autogenerated.yaml | 2 ++ test/cpp/end2end/BUILD | 1 + test/cpp/end2end/rls_end2end_test.cc | 3 +++ 4 files changed, 7 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index c6e292b0373..787db67baa0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -26337,6 +26337,7 @@ add_executable(rls_end2end_test ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/orca_load_report.grpc.pb.cc ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/orca_load_report.pb.h ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/orca_load_report.grpc.pb.h + test/core/event_engine/event_engine_test_utils.cc test/core/test_util/fake_stats_plugin.cc test/core/test_util/test_lb_policies.cc test/cpp/end2end/rls_end2end_test.cc diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 7e4f95f2932..dd845a6d959 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -17596,6 +17596,7 @@ targets: run: false language: c++ headers: + - test/core/event_engine/event_engine_test_utils.h - test/core/test_util/fake_stats_plugin.h - test/core/test_util/test_lb_policies.h - test/cpp/end2end/counted_service.h @@ -17608,6 +17609,7 @@ targets: - src/proto/grpc/testing/echo_messages.proto - src/proto/grpc/testing/simple_messages.proto - src/proto/grpc/testing/xds/v3/orca_load_report.proto + - test/core/event_engine/event_engine_test_utils.cc - test/core/test_util/fake_stats_plugin.cc - test/core/test_util/test_lb_policies.cc - test/cpp/end2end/rls_end2end_test.cc diff --git a/test/cpp/end2end/BUILD b/test/cpp/end2end/BUILD index 4e0b8ed3e55..3e01fe662f9 100644 --- a/test/cpp/end2end/BUILD +++ b/test/cpp/end2end/BUILD @@ -584,6 +584,7 @@ grpc_cc_test( "//src/proto/grpc/testing:echo_messages_proto", "//src/proto/grpc/testing:echo_proto", "//src/proto/grpc/testing/duplicate:echo_duplicate_proto", + "//test/core/event_engine:event_engine_test_utils", "//test/core/test_util:fake_stats_plugin", "//test/core/test_util:grpc_test_util", "//test/core/test_util:test_lb_policies", diff --git a/test/cpp/end2end/rls_end2end_test.cc b/test/cpp/end2end/rls_end2end_test.cc index 5eb5e41ac3e..189c4c2d34c 100644 --- a/test/cpp/end2end/rls_end2end_test.cc +++ b/test/cpp/end2end/rls_end2end_test.cc @@ -58,6 +58,7 @@ #include "src/proto/grpc/lookup/v1/rls.grpc.pb.h" #include "src/proto/grpc/lookup/v1/rls.pb.h" #include "src/proto/grpc/testing/echo.grpc.pb.h" +#include "test/core/event_engine/event_engine_test_utils.h" #include "test/core/test_util/fake_stats_plugin.h" #include "test/core/test_util/port.h" #include "test/core/test_util/resolve_localhost_ip46.h" @@ -175,6 +176,8 @@ class RlsEnd2endTest : public ::testing::Test { static void TearDownTestSuite() { grpc_shutdown_blocking(); + WaitForSingleOwner( + grpc_event_engine::experimental::GetDefaultEventEngine()); grpc_core::CoreConfiguration::Reset(); } From 540bfca8c6ead54436ea215bd0afec454329f610 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 7 May 2024 19:36:05 +0000 Subject: [PATCH 4/5] table-flip --- test/core/test_util/build.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/core/test_util/build.cc b/test/core/test_util/build.cc index 2ae07567d5b..36510696681 100644 --- a/test/core/test_util/build.cc +++ b/test/core/test_util/build.cc @@ -12,8 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "test/core/test_util/build.h" - // Define GRPC_BUILD_HAS_ASAN as 1 or 0 depending on if we're building under // ASAN. #if defined(__has_feature) From 6ce548590cdde80eb53639a400fa0f5af78caeb7 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 7 May 2024 19:42:52 +0000 Subject: [PATCH 5/5] x --- src/core/BUILD | 1 + src/core/server/server_interface.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/BUILD b/src/core/BUILD index 7b585215712..c0432c3f37f 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -3082,6 +3082,7 @@ grpc_cc_library( deps = [ "channel_args", "//:channelz", + "//:event_engine_base_hdrs", "//:gpr_platform", ], ) diff --git a/src/core/server/server_interface.h b/src/core/server/server_interface.h index 4523a3090ba..431ad369714 100644 --- a/src/core/server/server_interface.h +++ b/src/core/server/server_interface.h @@ -17,7 +17,7 @@ #ifndef GRPC_SRC_CORE_SERVER_SERVER_INTERFACE_H #define GRPC_SRC_CORE_SERVER_SERVER_INTERFACE_H -#include +#include #include #include "src/core/channelz/channelz.h"