Fix sanitizer builds (#2940)

* Make GrpcConnection safe to use with thread sanitizer

One additional lock acquisition per connection attempt isn't going to
kill us.

* Make tests pass with tsan and ubsan

Author: wilhuff

Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm

@@ -25,6 +25,7 @@
 #import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h"
 
 #include "Firestore/core/src/firebase/firestore/util/autoid.h"
+#include "Firestore/core/src/firebase/firestore/util/sanitizers.h"
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 
 using firebase::firestore::util::CreateAutoId;
@@ -404,9 +405,15 @@ - (void)testReasonableMemoryUsageForLotsOfMutations {
     const int64_t memoryUsedAfterCommitMb = GetCurrentMemoryUsedInMb();
     XCTAssertNotEqual(memoryUsedAfterCommitMb, -1);
     const int64_t memoryDeltaMb = memoryUsedAfterCommitMb - memoryUsedBeforeCommitMb;
+
+#if !defined(THREAD_SANITIZER) && !defined(ADDRESS_SANITIZER)
     // This by its nature cannot be a precise value. Runs on simulator seem to give an increase of
     // 10MB in debug mode pretty consistently. A regression would be on the scale of 500Mb.
+    //
+    // This check is disabled under the thread sanitizer because it introduces an overhead of
+    // 5x-10x.
     XCTAssertLessThan(memoryDeltaMb, 20);
+#endif
 
     [expectation fulfill];
   }];

Firestore/core/src/firebase/firestore/remote/grpc_connection.cc

@@ -17,6 +17,7 @@
 #include "Firestore/core/src/firebase/firestore/remote/grpc_connection.h"
 
 #include <algorithm>
+#include <mutex>  // NOLINT(build/c++11)
 #include <string>
 #include <utility>
 
@@ -66,15 +67,51 @@ struct HostConfig {
   bool use_insecure_channel = false;
 };
 
-using ConfigByHost = std::unordered_map<std::string, HostConfig>;
+class HostConfigMap {
+  using ConfigByHost = std::unordered_map<std::string, HostConfig>;
+  using Guard = std::lock_guard<std::mutex>;
+
+ public:
+  const HostConfig* _Nullable find(const std::string& host) const {
+    Guard guard{mutex_};
+    auto iter = map_.find(host);
+    if (iter == map_.end()) {
+      return nullptr;
+    } else {
+      return &(iter->second);
+    }
+  }
 
-ConfigByHost& Config() {
-  static ConfigByHost config_by_host_;
-  return config_by_host_;
-}
+  void UseTestCertificate(const std::string& host,
+                          const Path& certificate_path,
+                          const std::string& target_name) {
+    HARD_ASSERT(!host.empty(), "Empty host name");
+    HARD_ASSERT(!certificate_path.native_value().empty(),
+                "Empty path to test certificate");
+    HARD_ASSERT(!target_name.empty(), "Empty SSL target name");
+
+    Guard guard(mutex_);
+    HostConfig& host_config = map_[host];
+    host_config.certificate_path = certificate_path;
+    host_config.target_name = target_name;
+  }
+
+  void UseInsecureChannel(const std::string& host) {
+    HARD_ASSERT(!host.empty(), "Empty host name");
+
+    Guard guard(mutex_);
+    HostConfig& host_config = map_[host];
+    host_config.use_insecure_channel = true;
+  }
+
+ private:
+  ConfigByHost map_;
+  mutable std::mutex mutex_;
+};
 
-bool HasSpecialConfig(const std::string& host) {
-  return Config().find(host) != Config().end();
+HostConfigMap& Config() {
+  static HostConfigMap config_by_host_;
+  return config_by_host_;
 }
 
 }  // namespace
@@ -144,22 +181,21 @@ void GrpcConnection::EnsureActiveStub() {
 std::shared_ptr<grpc::Channel> GrpcConnection::CreateChannel() const {
   const std::string& host = database_info_->host();
 
-  if (!HasSpecialConfig(host)) {
+  const HostConfig* host_config = Config().find(host);
+  if (!host_config) {
     std::string root_certificate = LoadGrpcRootCertificate();
     return grpc::CreateChannel(host, CreateSslCredentials(root_certificate));
   }
 
-  const HostConfig& host_config = Config()[host];
-
   // For the case when `Settings.sslEnabled == false`.
-  if (host_config.use_insecure_channel) {
+  if (host_config->use_insecure_channel) {
     return grpc::CreateChannel(host, grpc::InsecureChannelCredentials());
   }
 
   // For tests only
   grpc::ChannelArguments args;
-  args.SetSslTargetNameOverride(host_config.target_name);
-  Path path = host_config.certificate_path;
+  args.SetSslTargetNameOverride(host_config->target_name);
+  Path path = host_config->certificate_path;
   StatusOr<std::string> test_certificate = ReadFile(path);
   HARD_ASSERT(test_certificate.ok(),
               StringFormat("Unable to open root certificates at file path %s",
@@ -241,21 +277,11 @@ void GrpcConnection::Unregister(GrpcCall* call) {
     const std::string& host,
     const Path& certificate_path,
     const std::string& target_name) {
-  HARD_ASSERT(!host.empty(), "Empty host name");
-  HARD_ASSERT(!certificate_path.native_value().empty(),
-              "Empty path to test certificate");
-  HARD_ASSERT(!target_name.empty(), "Empty SSL target name");
-
-  HostConfig& host_config = Config()[host];
-  host_config.certificate_path = certificate_path;
-  host_config.target_name = target_name;
+  Config().UseTestCertificate(host, certificate_path, target_name);
 }
 
 /*static*/ void GrpcConnection::UseInsecureChannel(const std::string& host) {
-  HARD_ASSERT(!host.empty(), "Empty host name");
-
-  HostConfig& host_config = Config()[host];
-  host_config.use_insecure_channel = true;
+  Config().UseInsecureChannel(host);
 }
 
 }  // namespace remote

Firestore/core/src/firebase/firestore/util/sanitizers.h

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2019 Google
+ *
+ * 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.
+ */
+
+#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_UTIL_SANITIZERS_H_
+#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_UTIL_SANITIZERS_H_
+
+// Clang feature detection compiler extension
+#if defined(__has_feature)
+#define HAS_FEATURE(FEATURE) __has_feature(FEATURE)
+#else
+#define HAS_FEATURE(FEATURE) 0
+#endif
+
+// Auto-detect the presence of sanitizers. Abseil does not do this yet, and
+// instead relies on users to pass e.g. -DADDRESS_SANITIZER on the command-line.
+
+// Clang and GCC 4.8
+#if !defined(ADDRESS_SANITIZER)
+#if HAS_FEATURE(address_sanitizer) || \
+    (defined(__GNUC__) && defined(__SANITIZE_ADDRESS__))
+#define ADDRESS_SANITIZER 1
+#endif
+#endif  // !defined(ADDRESS_SANITIZER)
+
+// Clang-only
+#if !defined(MEMORY_SANITIZER)
+#if HAS_FEATURE(memory_sanitizer)
+#define MEMORY_SANITIZER 1
+#endif
+#endif  // !defined(MEMORY_SANITIZER)
+
+// Clang and GCC 4.8+
+#if !defined(THREAD_SANITIZER)
+#if HAS_FEATURE(thread_sanitizer) || \
+    (defined(__GNUC__) && defined(__SANITIZE_THREAD__))
+#define THREAD_SANITIZER 1
+#endif
+#endif  // !defined(THREAD_SANITIZER)
+
+// There doesn't appear to be a __has_feature check for ubsan.
+
+#endif  // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_UTIL_SANITIZERS_H_