From 338c55978be62b372c20c8a7868f35a0276cdbe3 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 19 May 2023 17:26:11 -0700 Subject: [PATCH] [threading] Implement thready-tsan mode (#33193) This test mode tries to create threads wherever it legally can to maximize the chances of TSAN finding errors in our codebase. --------- Co-authored-by: ctiller --- CMakeLists.txt | 5 +- Makefile | 2 + build_autogenerated.yaml | 10 +++- config.m4 | 2 + config.w32 | 2 + gRPC-C++.podspec | 2 + gRPC-Core.podspec | 3 ++ grpc.gemspec | 2 + grpc.gyp | 3 ++ package.xml | 2 + src/core/BUILD | 17 +++++++ .../lib/event_engine/default_event_engine.cc | 14 +++++- .../thready_event_engine.cc | 10 ++-- .../thready_event_engine.h | 8 ++-- src/core/lib/promise/party.cc | 6 +-- src/python/grpcio/grpc_core_dependencies.py | 1 + test/core/event_engine/test_suite/BUILD | 2 +- .../thready_posix_event_engine_test.cc | 2 +- .../event_engine/thready_event_engine/BUILD | 33 ------------- tools/bazel.rc | 10 ++++ tools/doxygen/Doxyfile.c++.internal | 2 + tools/doxygen/Doxyfile.core.internal | 2 + .../linux/grpc_bazel_rbe_thready_tsan.cfg | 46 +++++++++++++++++++ .../grpc_bazel_rbe_thready_tsan.cfg | 41 +++++++++++++++++ 24 files changed, 178 insertions(+), 49 deletions(-) rename {test/core => src/core/lib}/event_engine/thready_event_engine/thready_event_engine.cc (94%) rename {test/core => src/core/lib}/event_engine/thready_event_engine/thready_event_engine.h (93%) delete mode 100644 test/core/event_engine/thready_event_engine/BUILD create mode 100644 tools/internal_ci/linux/grpc_bazel_rbe_thready_tsan.cfg create mode 100644 tools/internal_ci/linux/pull_request/grpc_bazel_rbe_thready_tsan.cfg diff --git a/CMakeLists.txt b/CMakeLists.txt index d6b552a4ec4..aa7602fdfca 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2085,6 +2085,7 @@ add_library(grpc src/core/lib/event_engine/thread_pool/original_thread_pool.cc src/core/lib/event_engine/thread_pool/thread_pool_factory.cc src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc + src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc src/core/lib/event_engine/time_util.cc src/core/lib/event_engine/trace.cc src/core/lib/event_engine/utils.cc @@ -2786,6 +2787,7 @@ add_library(grpc_unsecure src/core/lib/event_engine/thread_pool/original_thread_pool.cc src/core/lib/event_engine/thread_pool/thread_pool_factory.cc src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc + src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc src/core/lib/event_engine/time_util.cc src/core/lib/event_engine/trace.cc src/core/lib/event_engine/utils.cc @@ -4318,6 +4320,7 @@ add_library(grpc_authorization_provider src/core/lib/event_engine/thread_pool/original_thread_pool.cc src/core/lib/event_engine/thread_pool/thread_pool_factory.cc src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc + src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc src/core/lib/event_engine/time_util.cc src/core/lib/event_engine/trace.cc src/core/lib/event_engine/utils.cc @@ -11551,6 +11554,7 @@ add_executable(frame_test src/core/lib/event_engine/thread_pool/original_thread_pool.cc src/core/lib/event_engine/thread_pool/thread_pool_factory.cc src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc + src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc src/core/lib/event_engine/time_util.cc src/core/lib/event_engine/trace.cc src/core/lib/event_engine/utils.cc @@ -21496,7 +21500,6 @@ if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) test/core/event_engine/test_suite/tests/server_test.cc test/core/event_engine/test_suite/tests/timer_test.cc test/core/event_engine/test_suite/thready_posix_event_engine_test.cc - test/core/event_engine/thready_event_engine/thready_event_engine.cc third_party/googletest/googletest/src/gtest-all.cc third_party/googletest/googlemock/src/gmock-all.cc ) diff --git a/Makefile b/Makefile index 833daa6d5a2..f3fd553ebb2 100644 --- a/Makefile +++ b/Makefile @@ -1465,6 +1465,7 @@ LIBGRPC_SRC = \ src/core/lib/event_engine/thread_pool/original_thread_pool.cc \ src/core/lib/event_engine/thread_pool/thread_pool_factory.cc \ src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc \ + src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc \ src/core/lib/event_engine/time_util.cc \ src/core/lib/event_engine/trace.cc \ src/core/lib/event_engine/utils.cc \ @@ -2020,6 +2021,7 @@ LIBGRPC_UNSECURE_SRC = \ src/core/lib/event_engine/thread_pool/original_thread_pool.cc \ src/core/lib/event_engine/thread_pool/thread_pool_factory.cc \ src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc \ + src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc \ src/core/lib/event_engine/time_util.cc \ src/core/lib/event_engine/trace.cc \ src/core/lib/event_engine/utils.cc \ diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 6f6fa14c5ed..bde37db8ba8 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -714,6 +714,7 @@ libs: - src/core/lib/event_engine/thread_pool/original_thread_pool.h - src/core/lib/event_engine/thread_pool/thread_pool.h - src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.h + - src/core/lib/event_engine/thready_event_engine/thready_event_engine.h - src/core/lib/event_engine/time_util.h - src/core/lib/event_engine/trace.h - src/core/lib/event_engine/utils.h @@ -1513,6 +1514,7 @@ libs: - src/core/lib/event_engine/thread_pool/original_thread_pool.cc - src/core/lib/event_engine/thread_pool/thread_pool_factory.cc - src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc + - src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc - src/core/lib/event_engine/time_util.cc - src/core/lib/event_engine/trace.cc - src/core/lib/event_engine/utils.cc @@ -2091,6 +2093,7 @@ libs: - src/core/lib/event_engine/thread_pool/original_thread_pool.h - src/core/lib/event_engine/thread_pool/thread_pool.h - src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.h + - src/core/lib/event_engine/thready_event_engine/thready_event_engine.h - src/core/lib/event_engine/time_util.h - src/core/lib/event_engine/trace.h - src/core/lib/event_engine/utils.h @@ -2501,6 +2504,7 @@ libs: - src/core/lib/event_engine/thread_pool/original_thread_pool.cc - src/core/lib/event_engine/thread_pool/thread_pool_factory.cc - src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc + - src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc - src/core/lib/event_engine/time_util.cc - src/core/lib/event_engine/trace.cc - src/core/lib/event_engine/utils.cc @@ -3586,6 +3590,7 @@ libs: - src/core/lib/event_engine/thread_pool/original_thread_pool.h - src/core/lib/event_engine/thread_pool/thread_pool.h - src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.h + - src/core/lib/event_engine/thready_event_engine/thready_event_engine.h - src/core/lib/event_engine/time_util.h - src/core/lib/event_engine/trace.h - src/core/lib/event_engine/utils.h @@ -3876,6 +3881,7 @@ libs: - src/core/lib/event_engine/thread_pool/original_thread_pool.cc - src/core/lib/event_engine/thread_pool/thread_pool_factory.cc - src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc + - src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc - src/core/lib/event_engine/time_util.cc - src/core/lib/event_engine/trace.cc - src/core/lib/event_engine/utils.cc @@ -7524,6 +7530,7 @@ targets: - src/core/lib/event_engine/thread_pool/original_thread_pool.h - src/core/lib/event_engine/thread_pool/thread_pool.h - src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.h + - src/core/lib/event_engine/thready_event_engine/thready_event_engine.h - src/core/lib/event_engine/time_util.h - src/core/lib/event_engine/trace.h - src/core/lib/event_engine/utils.h @@ -7795,6 +7802,7 @@ targets: - src/core/lib/event_engine/thread_pool/original_thread_pool.cc - src/core/lib/event_engine/thread_pool/thread_pool_factory.cc - src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc + - src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc - src/core/lib/event_engine/time_util.cc - src/core/lib/event_engine/trace.cc - src/core/lib/event_engine/utils.cc @@ -12157,7 +12165,6 @@ targets: - test/core/event_engine/test_suite/tests/client_test.h - test/core/event_engine/test_suite/tests/server_test.h - test/core/event_engine/test_suite/tests/timer_test.h - - test/core/event_engine/thready_event_engine/thready_event_engine.h src: - test/core/event_engine/event_engine_test_utils.cc - test/core/event_engine/test_suite/event_engine_test_framework.cc @@ -12166,7 +12173,6 @@ targets: - test/core/event_engine/test_suite/tests/server_test.cc - test/core/event_engine/test_suite/tests/timer_test.cc - test/core/event_engine/test_suite/thready_posix_event_engine_test.cc - - test/core/event_engine/thready_event_engine/thready_event_engine.cc deps: - grpc_unsecure - grpc_test_util diff --git a/config.m4 b/config.m4 index 15080fe4dc6..557bbb23dce 100644 --- a/config.m4 +++ b/config.m4 @@ -548,6 +548,7 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/event_engine/thread_pool/original_thread_pool.cc \ src/core/lib/event_engine/thread_pool/thread_pool_factory.cc \ src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc \ + src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc \ src/core/lib/event_engine/time_util.cc \ src/core/lib/event_engine/trace.cc \ src/core/lib/event_engine/utils.cc \ @@ -1467,6 +1468,7 @@ if test "$PHP_GRPC" != "no"; then PHP_ADD_BUILD_DIR($ext_builddir/src/core/lib/event_engine/cf_engine) PHP_ADD_BUILD_DIR($ext_builddir/src/core/lib/event_engine/posix_engine) PHP_ADD_BUILD_DIR($ext_builddir/src/core/lib/event_engine/thread_pool) + PHP_ADD_BUILD_DIR($ext_builddir/src/core/lib/event_engine/thready_event_engine) PHP_ADD_BUILD_DIR($ext_builddir/src/core/lib/event_engine/windows) PHP_ADD_BUILD_DIR($ext_builddir/src/core/lib/event_engine/work_queue) PHP_ADD_BUILD_DIR($ext_builddir/src/core/lib/experiments) diff --git a/config.w32 b/config.w32 index cf4d88cd31b..4049ab7309b 100644 --- a/config.w32 +++ b/config.w32 @@ -513,6 +513,7 @@ if (PHP_GRPC != "no") { "src\\core\\lib\\event_engine\\thread_pool\\original_thread_pool.cc " + "src\\core\\lib\\event_engine\\thread_pool\\thread_pool_factory.cc " + "src\\core\\lib\\event_engine\\thread_pool\\work_stealing_thread_pool.cc " + + "src\\core\\lib\\event_engine\\thready_event_engine\\thready_event_engine.cc " + "src\\core\\lib\\event_engine\\time_util.cc " + "src\\core\\lib\\event_engine\\trace.cc " + "src\\core\\lib\\event_engine\\utils.cc " + @@ -1599,6 +1600,7 @@ if (PHP_GRPC != "no") { FSO.CreateFolder(base_dir+"\\ext\\grpc\\src\\core\\lib\\event_engine\\cf_engine"); FSO.CreateFolder(base_dir+"\\ext\\grpc\\src\\core\\lib\\event_engine\\posix_engine"); FSO.CreateFolder(base_dir+"\\ext\\grpc\\src\\core\\lib\\event_engine\\thread_pool"); + FSO.CreateFolder(base_dir+"\\ext\\grpc\\src\\core\\lib\\event_engine\\thready_event_engine"); FSO.CreateFolder(base_dir+"\\ext\\grpc\\src\\core\\lib\\event_engine\\windows"); FSO.CreateFolder(base_dir+"\\ext\\grpc\\src\\core\\lib\\event_engine\\work_queue"); FSO.CreateFolder(base_dir+"\\ext\\grpc\\src\\core\\lib\\experiments"); diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index 36ce9f32d49..b459c7c0a2a 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -785,6 +785,7 @@ Pod::Spec.new do |s| 'src/core/lib/event_engine/thread_pool/original_thread_pool.h', 'src/core/lib/event_engine/thread_pool/thread_pool.h', 'src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.h', + 'src/core/lib/event_engine/thready_event_engine/thready_event_engine.h', 'src/core/lib/event_engine/time_util.h', 'src/core/lib/event_engine/trace.h', 'src/core/lib/event_engine/utils.h', @@ -1822,6 +1823,7 @@ Pod::Spec.new do |s| 'src/core/lib/event_engine/thread_pool/original_thread_pool.h', 'src/core/lib/event_engine/thread_pool/thread_pool.h', 'src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.h', + 'src/core/lib/event_engine/thready_event_engine/thready_event_engine.h', 'src/core/lib/event_engine/time_util.h', 'src/core/lib/event_engine/trace.h', 'src/core/lib/event_engine/utils.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 85ac919aa76..5b61bce43a3 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -1223,6 +1223,8 @@ Pod::Spec.new do |s| 'src/core/lib/event_engine/thread_pool/thread_pool_factory.cc', 'src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc', 'src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.h', + 'src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc', + 'src/core/lib/event_engine/thready_event_engine/thready_event_engine.h', 'src/core/lib/event_engine/time_util.cc', 'src/core/lib/event_engine/time_util.h', 'src/core/lib/event_engine/trace.cc', @@ -2548,6 +2550,7 @@ Pod::Spec.new do |s| 'src/core/lib/event_engine/thread_pool/original_thread_pool.h', 'src/core/lib/event_engine/thread_pool/thread_pool.h', 'src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.h', + 'src/core/lib/event_engine/thready_event_engine/thready_event_engine.h', 'src/core/lib/event_engine/time_util.h', 'src/core/lib/event_engine/trace.h', 'src/core/lib/event_engine/utils.h', diff --git a/grpc.gemspec b/grpc.gemspec index 09dff566dde..cc5629fd796 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -1129,6 +1129,8 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/event_engine/thread_pool/thread_pool_factory.cc ) s.files += %w( src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc ) s.files += %w( src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.h ) + s.files += %w( src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc ) + s.files += %w( src/core/lib/event_engine/thready_event_engine/thready_event_engine.h ) s.files += %w( src/core/lib/event_engine/time_util.cc ) s.files += %w( src/core/lib/event_engine/time_util.h ) s.files += %w( src/core/lib/event_engine/trace.cc ) diff --git a/grpc.gyp b/grpc.gyp index 3b577fdd91e..a5a94315ddf 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -768,6 +768,7 @@ 'src/core/lib/event_engine/thread_pool/original_thread_pool.cc', 'src/core/lib/event_engine/thread_pool/thread_pool_factory.cc', 'src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc', + 'src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc', 'src/core/lib/event_engine/time_util.cc', 'src/core/lib/event_engine/trace.cc', 'src/core/lib/event_engine/utils.cc', @@ -1262,6 +1263,7 @@ 'src/core/lib/event_engine/thread_pool/original_thread_pool.cc', 'src/core/lib/event_engine/thread_pool/thread_pool_factory.cc', 'src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc', + 'src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc', 'src/core/lib/event_engine/time_util.cc', 'src/core/lib/event_engine/trace.cc', 'src/core/lib/event_engine/utils.cc', @@ -1779,6 +1781,7 @@ 'src/core/lib/event_engine/thread_pool/original_thread_pool.cc', 'src/core/lib/event_engine/thread_pool/thread_pool_factory.cc', 'src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc', + 'src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc', 'src/core/lib/event_engine/time_util.cc', 'src/core/lib/event_engine/trace.cc', 'src/core/lib/event_engine/utils.cc', diff --git a/package.xml b/package.xml index 1d0dbd52354..ff04fd18946 100644 --- a/package.xml +++ b/package.xml @@ -1111,6 +1111,8 @@ + + diff --git a/src/core/BUILD b/src/core/BUILD index 3000838876a..240e6c362ca 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -2237,6 +2237,22 @@ grpc_cc_library( ], ) +grpc_cc_library( + name = "thready_event_engine", + srcs = ["lib/event_engine/thready_event_engine/thready_event_engine.cc"], + hdrs = ["lib/event_engine/thready_event_engine/thready_event_engine.h"], + external_deps = [ + "absl/functional:any_invocable", + "absl/status", + "absl/status:statusor", + "absl/strings", + ], + deps = [ + "//:event_engine_base_hdrs", + "//:gpr", + ], +) + grpc_cc_library( name = "default_event_engine", srcs = [ @@ -2252,6 +2268,7 @@ grpc_cc_library( "default_event_engine_factory", "event_engine_trace", "no_destruct", + "thready_event_engine", "//:config", "//:debug_location", "//:event_engine_base_hdrs", diff --git a/src/core/lib/event_engine/default_event_engine.cc b/src/core/lib/event_engine/default_event_engine.cc index 0b2c9212275..c37e5163a38 100644 --- a/src/core/lib/event_engine/default_event_engine.cc +++ b/src/core/lib/event_engine/default_event_engine.cc @@ -32,6 +32,10 @@ #include "src/core/lib/gprpp/no_destruct.h" #include "src/core/lib/gprpp/sync.h" +#ifdef GRPC_MAXIMIZE_THREADYNESS +#include "src/core/lib/event_engine/thready_event_engine/thready_event_engine.h" // IWYU pragma: keep +#endif + namespace grpc_event_engine { namespace experimental { @@ -57,13 +61,21 @@ void EventEngineFactoryReset() { g_event_engine->reset(); } -std::unique_ptr CreateEventEngine() { +std::unique_ptr CreateEventEngineInner() { if (auto* factory = g_event_engine_factory.load()) { return (*factory)(); } return DefaultEventEngineFactory(); } +std::unique_ptr CreateEventEngine() { +#ifdef GRPC_MAXIMIZE_THREADYNESS + return std::make_unique(CreateEventEngineInner()); +#else + return CreateEventEngineInner(); +#endif +} + std::shared_ptr GetDefaultEventEngine( grpc_core::SourceLocation location) { grpc_core::MutexLock lock(&*g_mu); diff --git a/test/core/event_engine/thready_event_engine/thready_event_engine.cc b/src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc similarity index 94% rename from test/core/event_engine/thready_event_engine/thready_event_engine.cc rename to src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc index 3b613ce4c38..44c4ba98fd9 100644 --- a/test/core/event_engine/thready_event_engine/thready_event_engine.cc +++ b/src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc @@ -12,21 +12,25 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "test/core/event_engine/thready_event_engine/thready_event_engine.h" +#include + +#include "src/core/lib/event_engine/thready_event_engine/thready_event_engine.h" #include #include -#include #include #include #include "src/core/lib/gprpp/crash.h" +#include "src/core/lib/gprpp/thd.h" namespace grpc_event_engine { namespace experimental { void ThreadyEventEngine::Asynchronously(absl::AnyInvocable fn) { - std::thread([fn = std::move(fn)]() mutable { fn(); }).detach(); + grpc_core::Thread t("thready_event_engine", std::move(fn), nullptr, + grpc_core::Thread::Options().set_joinable(false)); + t.Start(); } absl::StatusOr> diff --git a/test/core/event_engine/thready_event_engine/thready_event_engine.h b/src/core/lib/event_engine/thready_event_engine/thready_event_engine.h similarity index 93% rename from test/core/event_engine/thready_event_engine/thready_event_engine.h rename to src/core/lib/event_engine/thready_event_engine/thready_event_engine.h index 66b2a916586..5a8b13863bf 100644 --- a/test/core/event_engine/thready_event_engine/thready_event_engine.h +++ b/src/core/lib/event_engine/thready_event_engine/thready_event_engine.h @@ -12,8 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef GRPC_TEST_CORE_EVENT_ENGINE_THREADY_EVENT_ENGINE_THREADY_EVENT_ENGINE_H -#define GRPC_TEST_CORE_EVENT_ENGINE_THREADY_EVENT_ENGINE_THREADY_EVENT_ENGINE_H +#ifndef GRPC_SRC_CORE_LIB_EVENT_ENGINE_THREADY_EVENT_ENGINE_THREADY_EVENT_ENGINE_H +#define GRPC_SRC_CORE_LIB_EVENT_ENGINE_THREADY_EVENT_ENGINE_THREADY_EVENT_ENGINE_H + +#include #include #include @@ -103,4 +105,4 @@ class ThreadyEventEngine final : public EventEngine { } // namespace experimental } // namespace grpc_event_engine -#endif // GRPC_TEST_CORE_EVENT_ENGINE_THREADY_EVENT_ENGINE_THREADY_EVENT_ENGINE_H +#endif // GRPC_SRC_CORE_LIB_EVENT_ENGINE_THREADY_EVENT_ENGINE_THREADY_EVENT_ENGINE_H diff --git a/src/core/lib/promise/party.cc b/src/core/lib/promise/party.cc index 6c7e38011de..6697fdfd9ca 100644 --- a/src/core/lib/promise/party.cc +++ b/src/core/lib/promise/party.cc @@ -29,9 +29,7 @@ #include "src/core/lib/promise/activity.h" #include "src/core/lib/promise/trace.h" -// #define GRPC_PARTY_MAXIMIZE_THREADS - -#ifdef GRPC_PARTY_MAXIMIZE_THREADS +#ifdef GRPC_MAXIMIZE_THREADYNESS #include "src/core/lib/gprpp/thd.h" // IWYU pragma: keep #include "src/core/lib/iomgr/exec_ctx.h" // IWYU pragma: keep #endif @@ -202,7 +200,7 @@ void Party::RunLocked() { PartyOver(); } }; -#ifdef GRPC_PARTY_MAXIMIZE_THREADS +#ifdef GRPC_MAXIMIZE_THREADYNESS Thread thd( "RunParty", [body]() { diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 9fe442684c0..6f3165a3152 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -522,6 +522,7 @@ CORE_SOURCE_FILES = [ 'src/core/lib/event_engine/thread_pool/original_thread_pool.cc', 'src/core/lib/event_engine/thread_pool/thread_pool_factory.cc', 'src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc', + 'src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc', 'src/core/lib/event_engine/time_util.cc', 'src/core/lib/event_engine/trace.cc', 'src/core/lib/event_engine/utils.cc', diff --git a/test/core/event_engine/test_suite/BUILD b/test/core/event_engine/test_suite/BUILD index 8e8592896cf..d3c3f6ac82c 100644 --- a/test/core/event_engine/test_suite/BUILD +++ b/test/core/event_engine/test_suite/BUILD @@ -65,12 +65,12 @@ grpc_cc_test( uses_polling = True, deps = [ "//src/core:posix_event_engine", + "//src/core:thready_event_engine", "//test/core/event_engine:event_engine_test_utils", "//test/core/event_engine/test_suite/posix:oracle_event_engine_posix", "//test/core/event_engine/test_suite/tests:client", "//test/core/event_engine/test_suite/tests:server", "//test/core/event_engine/test_suite/tests:timer", - "//test/core/event_engine/thready_event_engine", ], ) diff --git a/test/core/event_engine/test_suite/thready_posix_event_engine_test.cc b/test/core/event_engine/test_suite/thready_posix_event_engine_test.cc index 8d964cda6cd..f3807109b4b 100644 --- a/test/core/event_engine/test_suite/thready_posix_event_engine_test.cc +++ b/test/core/event_engine/test_suite/thready_posix_event_engine_test.cc @@ -18,12 +18,12 @@ #include #include "src/core/lib/event_engine/posix_engine/posix_engine.h" +#include "src/core/lib/event_engine/thready_event_engine/thready_event_engine.h" #include "test/core/event_engine/test_suite/event_engine_test_framework.h" #include "test/core/event_engine/test_suite/posix/oracle_event_engine_posix.h" #include "test/core/event_engine/test_suite/tests/client_test.h" #include "test/core/event_engine/test_suite/tests/server_test.h" #include "test/core/event_engine/test_suite/tests/timer_test.h" -#include "test/core/event_engine/thready_event_engine/thready_event_engine.h" #include "test/core/util/test_config.h" int main(int argc, char** argv) { diff --git a/test/core/event_engine/thready_event_engine/BUILD b/test/core/event_engine/thready_event_engine/BUILD deleted file mode 100644 index 468f5ec06ad..00000000000 --- a/test/core/event_engine/thready_event_engine/BUILD +++ /dev/null @@ -1,33 +0,0 @@ -# Copyright 2023 gRPC authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -load("//bazel:grpc_build_system.bzl", "grpc_cc_library", "grpc_package") - -licenses(["notice"]) - -grpc_package( - name = "test/core/event_engine/thready_event_engine", - visibility = "tests", -) - -grpc_cc_library( - name = "thready_event_engine", - srcs = ["thready_event_engine.cc"], - hdrs = ["thready_event_engine.h"], - deps = [ - "//:event_engine_base_hdrs", - "//src/core:default_event_engine", - "//src/core:time", - ], -) diff --git a/tools/bazel.rc b/tools/bazel.rc index 976ac748622..b5e3d74a51e 100644 --- a/tools/bazel.rc +++ b/tools/bazel.rc @@ -107,6 +107,16 @@ build:tsan --copt=-DGRPC_TSAN build:tsan --linkopt=-fsanitize=thread build:tsan --action_env=TSAN_OPTIONS=suppressions=test/core/util/tsan_suppressions.txt:halt_on_error=1:second_deadlock_stack=1 +# a TSAN build that tries to create new threads whenever possible +build:thready_tsan --strip=never +build:thready_tsan --copt=-fsanitize=thread +build:thready_tsan --copt=-fno-omit-frame-pointer +build:thready_tsan --copt=-DGPR_NO_DIRECT_SYSCALLS +build:thready_tsan --copt=-DGRPC_TSAN +build:thready_tsan --copt=-DGRPC_MAXIMIZE_THREADYNESS +build:thready_tsan --linkopt=-fsanitize=thread +build:thready_tsan --action_env=TSAN_OPTIONS=suppressions=test/core/util/tsan_suppressions.txt:halt_on_error=1:second_deadlock_stack=1 + build:tsan_macos --strip=never build:tsan_macos --copt=-fsanitize=thread build:tsan_macos --copt=-fno-omit-frame-pointer diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index ce9ee127043..7eed490f54c 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -2125,6 +2125,8 @@ src/core/lib/event_engine/thread_pool/thread_pool.h \ src/core/lib/event_engine/thread_pool/thread_pool_factory.cc \ src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc \ src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.h \ +src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc \ +src/core/lib/event_engine/thready_event_engine/thready_event_engine.h \ src/core/lib/event_engine/time_util.cc \ src/core/lib/event_engine/time_util.h \ src/core/lib/event_engine/trace.cc \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 5756876d498..a8c4e9d7131 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1903,6 +1903,8 @@ src/core/lib/event_engine/thread_pool/thread_pool.h \ src/core/lib/event_engine/thread_pool/thread_pool_factory.cc \ src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.cc \ src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.h \ +src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc \ +src/core/lib/event_engine/thready_event_engine/thready_event_engine.h \ src/core/lib/event_engine/time_util.cc \ src/core/lib/event_engine/time_util.h \ src/core/lib/event_engine/trace.cc \ diff --git a/tools/internal_ci/linux/grpc_bazel_rbe_thready_tsan.cfg b/tools/internal_ci/linux/grpc_bazel_rbe_thready_tsan.cfg new file mode 100644 index 00000000000..71a6ea62101 --- /dev/null +++ b/tools/internal_ci/linux/grpc_bazel_rbe_thready_tsan.cfg @@ -0,0 +1,46 @@ +# Copyright 2019 The gRPC Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Config file for the internal CI (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc/tools/internal_ci/linux/grpc_bazel_rbe.sh" +timeout_mins: 90 +action { + define_artifacts { + regex: "**/*sponge_log.*" + regex: "github/grpc/reports/**" + } +} + +gfile_resources: "/bigstore/grpc-testing-secrets/gcp_credentials/resultstore_api_key" + +bazel_setting { + # In order for Kokoro to recognize this as a bazel build and publish the bazel resultstore link, + # the bazel_setting section needs to be present and "upsalite_frontend_address" needs to be + # set. The rest of configuration from bazel_setting is unused (we configure everything when bazel + # command is invoked). + upsalite_frontend_address: "https://source.cloud.google.com" +} + +env_vars { + # flags will be passed to bazel invocation + key: "BAZEL_FLAGS" + value: "--cache_test_results=no --config=thready_tsan" +} + +env_vars { + key: "UPLOAD_TEST_RESULTS" + value: "true" +} diff --git a/tools/internal_ci/linux/pull_request/grpc_bazel_rbe_thready_tsan.cfg b/tools/internal_ci/linux/pull_request/grpc_bazel_rbe_thready_tsan.cfg new file mode 100644 index 00000000000..99abc9ad278 --- /dev/null +++ b/tools/internal_ci/linux/pull_request/grpc_bazel_rbe_thready_tsan.cfg @@ -0,0 +1,41 @@ +# Copyright 2019 The gRPC Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Config file for the internal CI (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc/tools/internal_ci/linux/grpc_bazel_rbe.sh" +timeout_mins: 90 +action { + define_artifacts { + regex: "**/*sponge_log.*" + regex: "github/grpc/reports/**" + } +} + +gfile_resources: "/bigstore/grpc-testing-secrets/gcp_credentials/resultstore_api_key" + +bazel_setting { + # In order for Kokoro to recognize this as a bazel build and publish the bazel resultstore link, + # the bazel_setting section needs to be present and "upsalite_frontend_address" needs to be + # set. The rest of configuration from bazel_setting is unused (we configure everything when bazel + # command is invoked). + upsalite_frontend_address: "https://source.cloud.google.com" +} + +env_vars { + # flags will be passed to bazel invocation + key: "BAZEL_FLAGS" + value: "--config=thready_tsan" +}