Merge branch 'master' into server_callback

include/grpcpp/impl/codegen/channel_interface.h

@@ -21,6 +21,7 @@
 
 #include <grpc/impl/codegen/connectivity_state.h>
 #include <grpcpp/impl/codegen/call.h>
+#include <grpcpp/impl/codegen/client_context.h>
 #include <grpcpp/impl/codegen/status.h>
 #include <grpcpp/impl/codegen/time.h>
 

include/grpcpp/impl/codegen/client_interceptor.h

@@ -21,7 +21,6 @@
 
 #include <vector>
 
-#include <grpc/impl/codegen/log.h>
 #include <grpcpp/impl/codegen/interceptor.h>
 #include <grpcpp/impl/codegen/string_ref.h>
 

include/grpcpp/impl/codegen/interceptor.h

@@ -21,13 +21,13 @@
 
 #include <grpc/impl/codegen/grpc_types.h>
 #include <grpcpp/impl/codegen/byte_buffer.h>
-#include <grpcpp/impl/codegen/channel_interface.h>
 #include <grpcpp/impl/codegen/config.h>
 #include <grpcpp/impl/codegen/core_codegen_interface.h>
 #include <grpcpp/impl/codegen/metadata_map.h>
 
 namespace grpc {
 
+class ChannelInterface;
 class Status;
 
 namespace experimental {

include/grpcpp/impl/codegen/server_interceptor.h

@@ -22,7 +22,6 @@
 #include <atomic>
 #include <vector>
 
-#include <grpc/impl/codegen/log.h>
 #include <grpcpp/impl/codegen/interceptor.h>
 #include <grpcpp/impl/codegen/string_ref.h>
 

include/grpcpp/impl/codegen/server_interface.h

@@ -333,7 +333,12 @@ class ServerInterface : public internal::CallHook {
   }
 
  private:
-  virtual const std::vector<
+  // EXPERIMENTAL
+  // Getter method for the vector of interceptor factory objects.
+  // Returns a nullptr (rather than being pure) since this is a new method and
+  // adding a new pure method to an interface would be a breaking change (even
+  // though this is private and non-API)
+  virtual std::vector<
       std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>*
   interceptor_creators() {
     return nullptr;

include/grpcpp/server.h

@@ -191,8 +191,7 @@ class Server : public ServerInterface, private GrpcLibraryCodegen {
   grpc_server* server() override { return server_; };
 
  private:
-  const std::vector<
+  std::vector<std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>*
-      std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>*
   interceptor_creators() override {
     return &interceptor_creators_;
   }
@@ -229,6 +228,14 @@ class Server : public ServerInterface, private GrpcLibraryCodegen {
 
   ServerInitializer* initializer();
 
+  // A vector of interceptor factory objects.
+  // This should be destroyed after health_check_service_ and this requirement
+  // is satisfied by declaring interceptor_creators_ before
+  // health_check_service_. (C++ mandates that member objects be destroyed in
+  // the reverse order of initialization.)
+  std::vector<std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>
+      interceptor_creators_;
+
   const int max_receive_message_size_;
 
   /// The following completion queues are ONLY used in case of Sync API
@@ -268,9 +275,6 @@ class Server : public ServerInterface, private GrpcLibraryCodegen {
   // A special handler for resource exhausted in sync case
   std::unique_ptr<internal::MethodHandler> resource_exhausted_handler_;
 
-  std::vector<std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>
-      interceptor_creators_;
-
   // callback_cq_ references the callbackable completion queue associated
   // with this server (if any). It is set on the first call to CallbackCQ().
   // It is _not owned_ by the server; ownership belongs with its internal

src/core/ext/filters/client_channel/client_channel.cc

@@ -2951,6 +2951,27 @@ static void apply_service_config_to_call_locked(grpc_call_element* elem) {
   }
 }
 
+// If the channel is in TRANSIENT_FAILURE and the call is not
+// wait_for_ready=true, fails the call and returns true.
+static bool fail_call_if_in_transient_failure(grpc_call_element* elem) {
+  channel_data* chand = static_cast<channel_data*>(elem->channel_data);
+  call_data* calld = static_cast<call_data*>(elem->call_data);
+  grpc_transport_stream_op_batch* batch = calld->pending_batches[0].batch;
+  if (grpc_connectivity_state_check(&chand->state_tracker) ==
+          GRPC_CHANNEL_TRANSIENT_FAILURE &&
+      (batch->payload->send_initial_metadata.send_initial_metadata_flags &
+       GRPC_INITIAL_METADATA_WAIT_FOR_READY) == 0) {
+    pending_batches_fail(
+        elem,
+        grpc_error_set_int(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
+                               "channel is in state TRANSIENT_FAILURE"),
+                           GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE),
+        true /* yield_call_combiner */);
+    return true;
+  }
+  return false;
+}
+
 // Invoked once resolver results are available.
 static void process_service_config_and_start_lb_pick_locked(
     grpc_call_element* elem) {
@@ -2958,6 +2979,9 @@ static void process_service_config_and_start_lb_pick_locked(
   // Only get service config data on the first attempt.
   if (GPR_LIKELY(calld->num_attempts_completed == 0)) {
     apply_service_config_to_call_locked(elem);
+    // Check this after applying service config, since it may have
+    // affected the call's wait_for_ready value.
+    if (fail_call_if_in_transient_failure(elem)) return;
   }
   // Start LB pick.
   grpc_core::LbPicker::StartLocked(elem);
@@ -3127,6 +3151,16 @@ static void start_pick_locked(void* arg, grpc_error* ignored) {
     // We do not yet have an LB policy, so wait for a resolver result.
     if (GPR_UNLIKELY(!chand->started_resolving)) {
       start_resolving_locked(chand);
+    } else {
+      // Normally, we want to do this check in
+      // process_service_config_and_start_lb_pick_locked(), so that we
+      // can honor the wait_for_ready setting in the service config.
+      // However, if the channel is in TRANSIENT_FAILURE at this point, that
+      // means that the resolver has returned a failure, so we're not going
+      // to get a service config right away.  In that case, we fail the
+      // call now based on the wait_for_ready value passed in from the
+      // application.
+      if (fail_call_if_in_transient_failure(elem)) return;
     }
     // Create a new waiter, which will delete itself when done.
     grpc_core::New<grpc_core::ResolverResultWaiter>(elem);

src/core/ext/filters/client_channel/health/health_check_client.cc

@@ -299,6 +299,11 @@ HealthCheckClient::CallState::~CallState() {
             health_check_client_.get(), this);
   }
   if (call_ != nullptr) GRPC_SUBCHANNEL_CALL_UNREF(call_, "call_ended");
+  for (size_t i = 0; i < GRPC_CONTEXT_COUNT; i++) {
+    if (context_[i].destroy != nullptr) {
+      context_[i].destroy(context_[i].value);
+    }
+  }
   // Unset the call combiner cancellation closure.  This has the
   // effect of scheduling the previously set cancellation closure, if
   // any, so that it can release any internal references it may be
@@ -346,6 +351,7 @@ void HealthCheckClient::CallState::StartCall() {
   }
   // Initialize payload and batch.
   memset(&batch_, 0, sizeof(batch_));
+  payload_.context = context_;
   batch_.payload = &payload_;
   // on_complete callback takes ref, handled manually.
   Ref(DEBUG_LOCATION, "on_complete").release();

src/core/ext/transport/chttp2/server/chttp2_server.cc

@@ -37,6 +37,7 @@
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/channel/handshaker.h"
 #include "src/core/lib/channel/handshaker_registry.h"
+#include "src/core/lib/gpr/host_port.h"
 #include "src/core/lib/iomgr/endpoint.h"
 #include "src/core/lib/iomgr/resolve_address.h"
 #include "src/core/lib/iomgr/resource_quota.h"
@@ -366,8 +367,14 @@ grpc_error* grpc_chttp2_server_add_port(grpc_server* server, const char* addr,
 
   arg = grpc_channel_args_find(args, GRPC_ARG_ENABLE_CHANNELZ);
   if (grpc_channel_arg_get_bool(arg, false)) {
+    char* host;
+    char* port;
+    gpr_split_host_port(addr, &host, &port);
+    // allocated host's ownership is passed to ListenSocketNode.
     state->channelz_listen_socket =
-        grpc_core::MakeRefCounted<grpc_core::channelz::ListenSocketNode>();
+        grpc_core::MakeRefCounted<grpc_core::channelz::ListenSocketNode>(
+            grpc_core::UniquePtr<char>(host), *port_num);
+    gpr_free(port);
     socket_uuid = state->channelz_listen_socket->uuid();
   }
 

src/core/lib/channel/channelz.cc

@@ -374,7 +374,8 @@ grpc_json* SocketNode::RenderJson() {
   return top_level_json;
 }
 
-ListenSocketNode::ListenSocketNode() : BaseNode(EntityType::kSocket) {}
+ListenSocketNode::ListenSocketNode(UniquePtr<char> host, int port)
+    : BaseNode(EntityType::kSocket), host_(std::move(host)), port_(port) {}
 
 grpc_json* ListenSocketNode::RenderJson() {
   // We need to track these three json objects to build our object
@@ -388,6 +389,21 @@ grpc_json* ListenSocketNode::RenderJson() {
   json_iterator = nullptr;
   json_iterator = grpc_json_add_number_string_child(json, json_iterator,
                                                     "socketId", uuid());
+  json = top_level_json;
+  json_iterator = nullptr;
+  json_iterator = grpc_json_create_child(json_iterator, json, "local", nullptr,
+                                         GRPC_JSON_OBJECT, false);
+  json = json_iterator;
+  json_iterator = nullptr;
+  json_iterator = grpc_json_create_child(json_iterator, json, "tcpip_address",
+                                         nullptr, GRPC_JSON_OBJECT, false);
+  json = json_iterator;
+  json_iterator = nullptr;
+  json_iterator =
+      grpc_json_add_number_string_child(json, json_iterator, "port", port_);
+  json_iterator = grpc_json_create_child(json_iterator, json, "ip_address",
+                                         host_.get(), GRPC_JSON_STRING, false);
+
   return top_level_json;
 }
 

src/core/lib/channel/channelz.h

@@ -268,10 +268,15 @@ class SocketNode : public BaseNode {
 // Handles channelz bookkeeping for listen sockets
 class ListenSocketNode : public BaseNode {
  public:
-  ListenSocketNode();
+  // ListenSocketNode takes ownership of host.
+  ListenSocketNode(UniquePtr<char> host, int port);
   ~ListenSocketNode() override {}
 
   grpc_json* RenderJson() override;
+
+ private:
+  UniquePtr<char> host_;
+  int port_;
 };
 
 // Creation functions

src/cpp/server/server_cc.cc

@@ -620,7 +620,8 @@ Server::Server(
     std::vector<
         std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>
         interceptor_creators)
-    : max_receive_message_size_(max_receive_message_size),
+    : interceptor_creators_(std::move(interceptor_creators)),
+      max_receive_message_size_(max_receive_message_size),
       sync_server_cqs_(std::move(sync_server_cqs)),
       started_(false),
       shutdown_(false),
@@ -628,8 +629,7 @@ Server::Server(
       has_generic_service_(false),
       server_(nullptr),
       server_initializer_(new ServerInitializer(this)),
-      health_check_service_disabled_(false),
+      health_check_service_disabled_(false) {
-      interceptor_creators_(std::move(interceptor_creators)) {
   g_gli_initializer.summon();
   gpr_once_init(&g_once_init_callbacks, InitGlobalCallbacks);
   global_callbacks_ = g_callbacks;

test/core/memory_usage/BUILD

@@ -0,0 +1,64 @@
+# Copyright 2017 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_cc_test", "grpc_cc_binary", "grpc_package")
+
+grpc_package(name = "test/core/memory_usage")
+
+licenses(["notice"])  # Apache v2
+
+load("//test/core/util:grpc_fuzzer.bzl", "grpc_fuzzer")
+
+grpc_cc_binary(
+    name = "client",
+    testonly = 1,
+    srcs = ["client.cc"],
+    language = "C++",
+    deps = [
+        "//:gpr",
+        "//:grpc",
+        "//test/core/util:gpr_test_util",
+        "//test/core/util:grpc_test_util",
+    ],
+)
+
+grpc_cc_binary(
+    name = "server",
+    testonly = 1,
+    srcs = ["server.cc"],
+    language = "C++",
+    deps = [
+        "//:gpr",
+        "//:grpc",
+        "//test/core/util:gpr_test_util",
+        "//test/core/util:grpc_test_util",
+        "//test/core/end2end:ssl_test_data"
+    ],
+)
+
+grpc_cc_test(
+    name = "memory_usage_test",
+    srcs = ["memory_usage_test.cc"],
+    language = "C++",
+    data = [
+    ":client",
+    ":server",
+    ],
+    deps = [
+        "//:gpr",
+        "//:grpc",
+        "//test/core/util:gpr_test_util",
+        "//test/core/util:grpc_test_util",
+    ],
+)

test/cpp/end2end/client_interceptors_end2end_test.cc

@@ -152,7 +152,9 @@ class HijackingInterceptor : public experimental::Interceptor {
       EchoRequest req;
       auto* buffer = methods->GetSendMessage();
       auto copied_buffer = *buffer;
-      SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req);
+      EXPECT_TRUE(
+          SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req)
+              .ok());
       EXPECT_EQ(req.message(), "Hello");
     }
     if (methods->QueryInterceptionHookPoint(
@@ -255,7 +257,9 @@ class HijackingInterceptorMakesAnotherCall : public experimental::Interceptor {
       EchoRequest req;
       auto* buffer = methods->GetSendMessage();
       auto copied_buffer = *buffer;
-      SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req);
+      EXPECT_TRUE(
+          SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req)
+              .ok());
       EXPECT_EQ(req.message(), "Hello");
       req_ = req;
       stub_ = grpc::testing::EchoTestService::NewStub(
@@ -367,7 +371,9 @@ class LoggingInterceptor : public experimental::Interceptor {
       EchoRequest req;
       auto* buffer = methods->GetSendMessage();
       auto copied_buffer = *buffer;
-      SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req);
+      EXPECT_TRUE(
+          SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req)
+              .ok());
       EXPECT_TRUE(req.message().find("Hello") == 0);
     }
     if (methods->QueryInterceptionHookPoint(

test/cpp/end2end/client_lb_end2end_test.cc

@@ -42,6 +42,9 @@
 #include "src/core/lib/gprpp/debug_location.h"
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/iomgr/tcp_client.h"
+#include "src/core/lib/security/credentials/fake/fake_credentials.h"
+#include "src/cpp/client/secure_credentials.h"
+#include "src/cpp/server/secure_server_credentials.h"
 
 #include "src/proto/grpc/testing/echo.grpc.pb.h"
 #include "test/core/util/port.h"
@@ -207,19 +210,22 @@ class ClientLbEnd2endTest : public ::testing::Test {
     }  // else, default to pick first
     args.SetPointer(GRPC_ARG_FAKE_RESOLVER_RESPONSE_GENERATOR,
                     response_generator_.get());
-    return CreateCustomChannel("fake:///", InsecureChannelCredentials(), args);
+    std::shared_ptr<ChannelCredentials> creds(new SecureChannelCredentials(
+        grpc_fake_transport_security_credentials_create()));
+    return CreateCustomChannel("fake:///", std::move(creds), args);
   }
 
   bool SendRpc(
       const std::unique_ptr<grpc::testing::EchoTestService::Stub>& stub,
       EchoResponse* response = nullptr, int timeout_ms = 1000,
-      Status* result = nullptr) {
+      Status* result = nullptr, bool wait_for_ready = false) {
     const bool local_response = (response == nullptr);
     if (local_response) response = new EchoResponse;
     EchoRequest request;
     request.set_message(kRequestMessage_);
     ClientContext context;
     context.set_deadline(grpc_timeout_milliseconds_to_deadline(timeout_ms));
+    if (wait_for_ready) context.set_wait_for_ready(true);
     Status status = stub->Echo(&context, request, response);
     if (result != nullptr) *result = status;
     if (local_response) delete response;
@@ -228,10 +234,11 @@ class ClientLbEnd2endTest : public ::testing::Test {
 
   void CheckRpcSendOk(
       const std::unique_ptr<grpc::testing::EchoTestService::Stub>& stub,
-      const grpc_core::DebugLocation& location) {
+      const grpc_core::DebugLocation& location, bool wait_for_ready = false) {
     EchoResponse response;
     Status status;
-    const bool success = SendRpc(stub, &response, 2000, &status);
+    const bool success =
+        SendRpc(stub, &response, 2000, &status, wait_for_ready);
     ASSERT_TRUE(success) << "From " << location.file() << ":" << location.line()
                          << "\n"
                          << "Error: " << status.error_message() << " "
@@ -275,8 +282,9 @@ class ClientLbEnd2endTest : public ::testing::Test {
       std::ostringstream server_address;
       server_address << server_host << ":" << port_;
       ServerBuilder builder;
-      builder.AddListeningPort(server_address.str(),
+      std::shared_ptr<ServerCredentials> creds(new SecureServerCredentials(
-                               InsecureServerCredentials());
+          grpc_fake_transport_security_server_credentials_create()));
+      builder.AddListeningPort(server_address.str(), std::move(creds));
       builder.RegisterService(&service_);
       server_ = builder.BuildAndStart();
       std::lock_guard<std::mutex> lock(*mu);
@@ -306,7 +314,7 @@ class ClientLbEnd2endTest : public ::testing::Test {
       if (ignore_failure) {
         SendRpc(stub);
       } else {
-        CheckRpcSendOk(stub, location);
+        CheckRpcSendOk(stub, location, true);
       }
     } while (servers_[server_idx]->service_.request_count() == 0);
     ResetCounters();
@@ -518,7 +526,7 @@ TEST_F(ClientLbEnd2endTest, PickFirstUpdates) {
   do {
     channel_state = channel->GetState(true /* try to connect */);
   } while (channel_state == GRPC_CHANNEL_READY);
-  GPR_ASSERT(channel_state != GRPC_CHANNEL_READY);
+  ASSERT_NE(channel_state, GRPC_CHANNEL_READY);
   servers_[0]->service_.ResetCounters();
 
   // Next update introduces servers_[1], making the channel recover.
@@ -835,7 +843,7 @@ TEST_F(ClientLbEnd2endTest, RoundRobinUpdates) {
   do {
     channel_state = channel->GetState(true /* try to connect */);
   } while (channel_state == GRPC_CHANNEL_READY);
-  GPR_ASSERT(channel_state != GRPC_CHANNEL_READY);
+  ASSERT_NE(channel_state, GRPC_CHANNEL_READY);
   servers_[0]->service_.ResetCounters();
 
   // Next update introduces servers_[1], making the channel recover.
@@ -844,7 +852,7 @@ TEST_F(ClientLbEnd2endTest, RoundRobinUpdates) {
   SetNextResolution(ports);
   WaitForServer(stub, 1, DEBUG_LOCATION);
   channel_state = channel->GetState(false /* try to connect */);
-  GPR_ASSERT(channel_state == GRPC_CHANNEL_READY);
+  ASSERT_EQ(channel_state, GRPC_CHANNEL_READY);
 
   // Check LB policy name for the channel.
   EXPECT_EQ("round_robin", channel->GetLoadBalancingPolicyName());
@@ -954,7 +962,7 @@ TEST_F(ClientLbEnd2endTest, RoundRobinReresolve) {
     if (SendRpc(stub)) break;
     now = gpr_now(GPR_CLOCK_MONOTONIC);
   }
-  GPR_ASSERT(gpr_time_cmp(deadline, now) > 0);
+  ASSERT_GT(gpr_time_cmp(deadline, now), 0);
 }
 
 TEST_F(ClientLbEnd2endTest, RoundRobinSingleReconnect) {

test/cpp/end2end/grpclb_end2end_test.cc

@@ -539,13 +539,15 @@ class GrpclbEnd2endTest : public ::testing::Test {
     balancers_.at(i)->add_response(response, delay_ms);
   }
 
-  Status SendRpc(EchoResponse* response = nullptr, int timeout_ms = 1000) {
+  Status SendRpc(EchoResponse* response = nullptr, int timeout_ms = 1000,
+                 bool wait_for_ready = false) {
     const bool local_response = (response == nullptr);
     if (local_response) response = new EchoResponse;
     EchoRequest request;
     request.set_message(kRequestMessage_);
     ClientContext context;
     context.set_deadline(grpc_timeout_milliseconds_to_deadline(timeout_ms));
+    if (wait_for_ready) context.set_wait_for_ready(true);
     Status status = stub_->Echo(&context, request, response);
     if (local_response) delete response;
     return status;
@@ -1366,7 +1368,7 @@ TEST_F(SingleBalancerTest, DropAllFirst) {
           {}, {{"rate_limiting", num_of_drop_by_rate_limiting_addresses},
                {"load_balancing", num_of_drop_by_load_balancing_addresses}}),
       0);
-  const Status status = SendRpc();
+  const Status status = SendRpc(nullptr, 1000, true);
   EXPECT_FALSE(status.ok());
   EXPECT_EQ(status.error_message(), "Call dropped by load balancing policy");
 }
@@ -1391,7 +1393,7 @@ TEST_F(SingleBalancerTest, DropAll) {
   // fail.
   Status status;
   do {
-    status = SendRpc();
+    status = SendRpc(nullptr, 1000, true);
   } while (status.ok());
   EXPECT_FALSE(status.ok());
   EXPECT_EQ(status.error_message(), "Call dropped by load balancing policy");

test/cpp/end2end/server_interceptors_end2end_test.cc

@@ -103,7 +103,9 @@ class LoggingInterceptor : public experimental::Interceptor {
       EchoRequest req;
       auto* buffer = methods->GetSendMessage();
       auto copied_buffer = *buffer;
-      SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req);
+      EXPECT_TRUE(
+          SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req)
+              .ok());
       EXPECT_TRUE(req.message().find("Hello") == 0);
     }
     if (methods->QueryInterceptionHookPoint(

tools/remote_build/README.md

@@ -28,3 +28,6 @@ Sanitizer runs (asan, msan, tsan, ubsan):
 # manual run of bazel tests remotely on Foundry with given sanitizer
 bazel --bazelrc=tools/remote_build/manual.bazelrc test --config=asan //test/...
 ```
+
+Available command line options can be found in
+[Bazel command line reference](https://docs.bazel.build/versions/master/command-line-reference.html)

tools/remote_build/kokoro.bazelrc

@@ -36,5 +36,3 @@ build --test_env=USER=anon
 build --jobs=200
 build --test_output=errors
 build --keep_going=true
-build --remote_accept_cached=true
-build --remote_local_fallback=true

tools/remote_build/manual.bazelrc

@@ -40,3 +40,6 @@ build --jobs=100
 # TODO(jtattermusch): this should be part of the common config
 # but currently sanitizers use different test_timeout values
 build --test_timeout=300,450,1200,3600
+
+# print output for tests that fail (default is "summary")
+build --test_output=errors

tools/remote_build/rbe_common.bazelrc

@@ -49,12 +49,6 @@ build:asan --copt=-gmlt
 # TODO(jtattermusch): use more reasonable test timeout
 build:asan --test_timeout=3600
 build:asan --test_tag_filters=-qps_json_driver,-json_run_localhost
-# TODO: revisit these from bazel.rc:
-#build:asan --copt=-O0
-#build:asan --copt=-fno-omit-frame-pointer
-#build:asan --copt=-DGPR_NO_DIRECT_SYSCALLS
-#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
 
 # memory sanitizer: most settings are already in %workspace%/.bazelrc
 # we only need a few additional ones that are Foundry specific
@@ -62,7 +56,7 @@ build:msan --copt=-gmlt
 # TODO(jtattermusch): use more reasonable test timeout
 build:msan --test_timeout=3600
 build:msan --cxxopt=--stdlib=libc++
-# TODO(jtattermusch): setting LD_LIBRARY_PATH is necessary
+# setting LD_LIBRARY_PATH is necessary
 # to avoid "libc++.so.1: cannot open shared object file"
 build:msan --action_env=LD_LIBRARY_PATH=/usr/local/lib
 build:msan --host_crosstool_top=@com_github_bazelbuild_bazeltoolchains//configs/ubuntu16_04_clang/1.0/bazel_0.16.1/default:toolchain
@@ -75,23 +69,14 @@ build:tsan --copt=-gmlt
 # TODO(jtattermusch): use more reasonable test timeout
 build:tsan --test_timeout=3600
 build:tsan --test_tag_filters=-qps_json_driver,-json_run_localhost
-# TODO: revisit these from bazel.rc:
-#build:tsan --copt=-fno-omit-frame-pointer
-#build:tsan --copt=-DGPR_NO_DIRECT_SYSCALLS
-#build:tsan --copt=-DGRPC_TSAN
 
 # undefined behavior sanitizer: most settings are already in %workspace%/.bazelrc
 # we only need a few additional ones that are Foundry specific
 build:ubsan --copt=-gmlt
 # TODO(jtattermusch): use more reasonable test timeout
 build:ubsan --test_timeout=3600
-# TODO: revisit these from bazel.rc:
+# override the config-agnostic crosstool_top
-#build:ubsan --copt=-fno-omit-frame-pointer
+--crosstool_top=@com_github_bazelbuild_bazeltoolchains//configs/experimental/ubuntu16_04_clang/1.0/bazel_0.16.1/ubsan:toolchain
-#build:ubsan --copt=-DGRPC_UBSAN
-#build:ubsan --copt=-DNDEBUG
-#build:ubsan --copt=-fno-sanitize=function,vptr
-# TODO: revisit this from grpc_ubsan_on_foundry.sh:
-#--crosstool_top=@com_github_bazelbuild_bazeltoolchains//configs/experimental/ubuntu16_04_clang/1.0/bazel_0.15.0/ubsan:toolchain
 # TODO(jtattermusch): remove this once Foundry adds the env to the docker image.
 # ubsan needs symbolizer to work properly, otherwise the suppression file doesn't work
 # and we get test failures.