diff --git a/include/grpc/impl/codegen/port_platform.h b/include/grpc/impl/codegen/port_platform.h index d7294d59d41..c0ca9f43716 100644 --- a/include/grpc/impl/codegen/port_platform.h +++ b/include/grpc/impl/codegen/port_platform.h @@ -394,6 +394,18 @@ #endif #endif /* GPR_NO_AUTODETECT_PLATFORM */ +#if defined(GPR_BACKWARDS_COMPATIBILITY_MODE) +/* + * For backward compatibility mode, reset _FORTIFY_SOURCE to prevent + * a library from having non-standard symbols such as __asprintf_chk. + * This helps non-glibc systems such as alpine using musl to find symbols. + */ +#if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 0 +#undef _FORTIFY_SOURCE +#define _FORTIFY_SOURCE 0 +#endif +#endif + /* * There are platforms for which TLS should not be used even though the * compiler makes it seem like it's supported (Android NDK < r12b for example). diff --git a/include/grpcpp/impl/codegen/callback_common.h b/include/grpcpp/impl/codegen/callback_common.h index a3c8c41246c..6170170b978 100644 --- a/include/grpcpp/impl/codegen/callback_common.h +++ b/include/grpcpp/impl/codegen/callback_common.h @@ -201,9 +201,11 @@ class CallbackWithSuccessTag void* ignored = ops_; // Allow a "false" return value from FinalizeResult to silence the // callback, just as it silences a CQ tag in the async cases +#ifndef NDEBUG auto* ops = ops_; +#endif bool do_callback = ops_->FinalizeResult(&ignored, &ok); - GPR_CODEGEN_ASSERT(ignored == ops); + GPR_CODEGEN_DEBUG_ASSERT(ignored == ops); if (do_callback) { CatchingCallback(func_, ok); diff --git a/include/grpcpp/impl/codegen/core_codegen_interface.h b/include/grpcpp/impl/codegen/core_codegen_interface.h index 02b5033c51f..8a7ac0ad452 100644 --- a/include/grpcpp/impl/codegen/core_codegen_interface.h +++ b/include/grpcpp/impl/codegen/core_codegen_interface.h @@ -144,7 +144,7 @@ extern CoreCodegenInterface* g_core_codegen_interface; /// Codegen specific version of \a GPR_ASSERT. #define GPR_CODEGEN_ASSERT(x) \ do { \ - if (!(x)) { \ + if (GPR_UNLIKELY(!(x))) { \ grpc::g_core_codegen_interface->assert_fail(#x, __FILE__, __LINE__); \ } \ } while (0) diff --git a/src/core/lib/gprpp/thd_posix.cc b/src/core/lib/gprpp/thd_posix.cc index 99197cbbfa9..2c3ae34ed44 100644 --- a/src/core/lib/gprpp/thd_posix.cc +++ b/src/core/lib/gprpp/thd_posix.cc @@ -49,11 +49,10 @@ struct thd_arg { bool tracked; }; -// TODO(yunjiaw): move this to a function-level static, or remove the use of a -// non-constexpr initializer when possible -const size_t page_size = static_cast(sysconf(_SC_PAGESIZE)); - size_t RoundUpToPageSize(size_t size) { + // TODO(yunjiaw): Change this variable (page_size) to a function-level static + // when possible + size_t page_size = static_cast(sysconf(_SC_PAGESIZE)); return (size + page_size - 1) & ~(page_size - 1); } diff --git a/src/csharp/Grpc.Core/Version.csproj.include b/src/csharp/Grpc.Core/Version.csproj.include deleted file mode 100755 index e24afdad605..00000000000 --- a/src/csharp/Grpc.Core/Version.csproj.include +++ /dev/null @@ -1,7 +0,0 @@ - - - - 1.19.1 - 3.8.0 - - diff --git a/src/csharp/build/dependencies.props b/src/csharp/build/dependencies.props index d9bf90ff007..a91b5e19414 100644 --- a/src/csharp/build/dependencies.props +++ b/src/csharp/build/dependencies.props @@ -2,6 +2,6 @@ 1.23.0-dev - 3.7.0 + 3.8.0 diff --git a/templates/src/csharp/build/dependencies.props.template b/templates/src/csharp/build/dependencies.props.template index 0ed9018a49e..88470df2898 100755 --- a/templates/src/csharp/build/dependencies.props.template +++ b/templates/src/csharp/build/dependencies.props.template @@ -4,6 +4,6 @@ ${settings.csharp_version} - 3.7.0 + 3.8.0 diff --git a/test/cpp/end2end/client_callback_end2end_test.cc b/test/cpp/end2end/client_callback_end2end_test.cc index 8cf6def1073..a54158ac1d1 100644 --- a/test/cpp/end2end/client_callback_end2end_test.cc +++ b/test/cpp/end2end/client_callback_end2end_test.cc @@ -374,6 +374,57 @@ TEST_P(ClientCallbackEnd2endTest, SimpleRpc) { SendRpcs(1, false); } +TEST_P(ClientCallbackEnd2endTest, SimpleRpcUnderLockNested) { + MAYBE_SKIP_TEST; + ResetStub(); + std::mutex mu1, mu2, mu3; + std::condition_variable cv; + bool done = false; + EchoRequest request1, request2, request3; + request1.set_message("Hello locked world1."); + request2.set_message("Hello locked world2."); + request3.set_message("Hello locked world3."); + EchoResponse response1, response2, response3; + ClientContext cli_ctx1, cli_ctx2, cli_ctx3; + { + std::lock_guard l(mu1); + stub_->experimental_async()->Echo( + &cli_ctx1, &request1, &response1, + [this, &mu1, &mu2, &mu3, &cv, &done, &request1, &request2, &request3, + &response1, &response2, &response3, &cli_ctx2, &cli_ctx3](Status s1) { + std::lock_guard l1(mu1); + EXPECT_TRUE(s1.ok()); + EXPECT_EQ(request1.message(), response1.message()); + // start the second level of nesting + std::unique_lock l2(mu2); + this->stub_->experimental_async()->Echo( + &cli_ctx2, &request2, &response2, + [this, &mu2, &mu3, &cv, &done, &request2, &request3, &response2, + &response3, &cli_ctx3](Status s2) { + std::lock_guard l2(mu2); + EXPECT_TRUE(s2.ok()); + EXPECT_EQ(request2.message(), response2.message()); + // start the third level of nesting + std::lock_guard l3(mu3); + stub_->experimental_async()->Echo( + &cli_ctx3, &request3, &response3, + [&mu3, &cv, &done, &request3, &response3](Status s3) { + std::lock_guard l(mu3); + EXPECT_TRUE(s3.ok()); + EXPECT_EQ(request3.message(), response3.message()); + done = true; + cv.notify_all(); + }); + }); + }); + } + + std::unique_lock l(mu3); + while (!done) { + cv.wait(l); + } +} + TEST_P(ClientCallbackEnd2endTest, SimpleRpcUnderLock) { MAYBE_SKIP_TEST; ResetStub(); diff --git a/test/cpp/end2end/generic_end2end_test.cc b/test/cpp/end2end/generic_end2end_test.cc index c2807310aa4..f9f68bbe023 100644 --- a/test/cpp/end2end/generic_end2end_test.cc +++ b/test/cpp/end2end/generic_end2end_test.cc @@ -62,6 +62,7 @@ class GenericEnd2endTest : public ::testing::Test { GenericEnd2endTest() : server_host_("localhost") {} void SetUp() override { + shut_down_ = false; int port = grpc_pick_unused_port_or_die(); server_address_ << server_host_ << ":" << port; // Setup server @@ -77,21 +78,26 @@ class GenericEnd2endTest : public ::testing::Test { server_ = builder.BuildAndStart(); } - void TearDown() override { - server_->Shutdown(); - void* ignored_tag; - bool ignored_ok; - cli_cq_.Shutdown(); - srv_cq_->Shutdown(); - while (cli_cq_.Next(&ignored_tag, &ignored_ok)) - ; - while (srv_cq_->Next(&ignored_tag, &ignored_ok)) - ; + void ShutDownServerAndCQs() { + if (!shut_down_) { + server_->Shutdown(); + void* ignored_tag; + bool ignored_ok; + cli_cq_.Shutdown(); + srv_cq_->Shutdown(); + while (cli_cq_.Next(&ignored_tag, &ignored_ok)) + ; + while (srv_cq_->Next(&ignored_tag, &ignored_ok)) + ; + shut_down_ = true; + } } + void TearDown() override { ShutDownServerAndCQs(); } void ResetStub() { std::shared_ptr channel = grpc::CreateChannel( server_address_.str(), InsecureChannelCredentials()); + stub_ = grpc::testing::EchoTestService::NewStub(channel); generic_stub_.reset(new GenericStub(channel)); } @@ -177,6 +183,54 @@ class GenericEnd2endTest : public ::testing::Test { } } + // Return errors to up to one call that comes in on the supplied completion + // queue, until the CQ is being shut down (and therefore we can no longer + // enqueue further events). + void DriveCompletionQueue() { + enum class Event : uintptr_t { + kCallReceived, + kResponseSent, + }; + // Request the call, but only if the main thread hasn't beaten us to + // shutting down the CQ. + grpc::GenericServerContext server_context; + grpc::GenericServerAsyncReaderWriter reader_writer(&server_context); + + { + std::lock_guard lock(shutting_down_mu_); + if (!shutting_down_) { + generic_service_.RequestCall( + &server_context, &reader_writer, srv_cq_.get(), srv_cq_.get(), + reinterpret_cast(Event::kCallReceived)); + } + } + // Process events. + { + Event event; + bool ok; + while (srv_cq_->Next(reinterpret_cast(&event), &ok)) { + std::lock_guard lock(shutting_down_mu_); + if (shutting_down_) { + // The main thread has started shutting down. Simply continue to drain + // events. + continue; + } + + switch (event) { + case Event::kCallReceived: + reader_writer.Finish( + ::grpc::Status(::grpc::StatusCode::UNIMPLEMENTED, "go away"), + reinterpret_cast(Event::kResponseSent)); + break; + + case Event::kResponseSent: + // We are done. + break; + } + } + } + } + CompletionQueue cli_cq_; std::unique_ptr srv_cq_; std::unique_ptr stub_; @@ -185,6 +239,9 @@ class GenericEnd2endTest : public ::testing::Test { AsyncGenericService generic_service_; const grpc::string server_host_; std::ostringstream server_address_; + bool shutting_down_; + bool shut_down_; + std::mutex shutting_down_mu_; }; TEST_F(GenericEnd2endTest, SimpleRpc) { @@ -330,6 +387,29 @@ TEST_F(GenericEnd2endTest, Deadline) { gpr_time_from_seconds(10, GPR_TIMESPAN))); } +TEST_F(GenericEnd2endTest, ShortDeadline) { + ResetStub(); + + ClientContext cli_ctx; + EchoRequest request; + EchoResponse response; + + shutting_down_ = false; + std::thread driver([this] { DriveCompletionQueue(); }); + + request.set_message(""); + cli_ctx.set_deadline(gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), + gpr_time_from_micros(500, GPR_TIMESPAN))); + Status s = stub_->Echo(&cli_ctx, request, &response); + EXPECT_FALSE(s.ok()); + { + std::lock_guard lock(shutting_down_mu_); + shutting_down_ = true; + } + ShutDownServerAndCQs(); + driver.join(); +} + } // namespace } // namespace testing } // namespace grpc diff --git a/tools/bazel.rc b/tools/bazel.rc index 411457a7cf5..e2d49355d8f 100644 --- a/tools/bazel.rc +++ b/tools/bazel.rc @@ -13,9 +13,6 @@ build:opt --copt=-Wframe-larger-than=16384 build:dbg --compilation_mode=dbg build:asan --strip=never -# Workaround for https://github.com/bazelbuild/bazel/issues/6932 -build:asan --copt -Wno-macro-redefined -build:asan --copt -D_FORTIFY_SOURCE=0 build:asan --copt=-fsanitize=address build:asan --copt=-O0 build:asan --copt=-fno-omit-frame-pointer @@ -25,6 +22,27 @@ build:asan --linkopt=-fsanitize=address build:asan --action_env=ASAN_OPTIONS=detect_leaks=1:color=always build:asan --action_env=LSAN_OPTIONS=suppressions=test/core/util/lsan_suppressions.txt:report_objects=1 +# We have a separate ASAN config for Mac OS to workaround a couple of bugs: +# 1. https://github.com/bazelbuild/bazel/issues/6932 +# _FORTIFY_SOURCE=1 is enabled by default on Mac OS, which breaks ASAN. +# We workaround it by setting _FORTIFY_SOURCE=0 and ignoring macro redefined +# warnings. +# 2. https://github.com/google/sanitizers/issues/1026 +# LSAN is not supported by the version of Clang that ships with Mac OS, so +# we disable it. +build:asan_macos --strip=never +build:asan_macos --copt=-fsanitize=address +build:asan_macos --copt -Wno-macro-redefined +build:asan_macos --copt -D_FORTIFY_SOURCE=0 +build:asan_macos --copt=-fsanitize=address +build:asan_macos --copt=-O0 +build:asan_macos --copt=-fno-omit-frame-pointer +build:asan_macos --copt=-DGPR_NO_DIRECT_SYSCALLS +build:asan_macos --copt=-DADDRESS_SANITIZER # used by absl +build:asan_macos --linkopt=-fsanitize=address +build:asan_macos --action_env=ASAN_OPTIONS=detect_leaks=0 + + build:msan --strip=never build:msan --copt=-fsanitize=memory build:msan --copt=-O0 diff --git a/tools/internal_ci/macos/grpc_cfstream_asan.cfg b/tools/internal_ci/macos/grpc_cfstream_asan.cfg index ceff78a4399..ac8a8b63ff6 100644 --- a/tools/internal_ci/macos/grpc_cfstream_asan.cfg +++ b/tools/internal_ci/macos/grpc_cfstream_asan.cfg @@ -18,6 +18,6 @@ build_file: "grpc/tools/internal_ci/macos/grpc_run_bazel_tests.sh" env_vars { key: "RUN_TESTS_FLAGS" - value: "--config=asan" + value: "--config=asan_macos" }