Mila/test mieq race issue

Firestore/core/src/core/query.cc

@@ -92,7 +92,7 @@ absl::optional<Operator> Query::FindOpInsideFilters(
 }
 
 const std::vector<OrderBy>& Query::normalized_order_bys() const {
-  return memoized_normalized_order_bys_->memoize([&]() {
+  if (memoized_normalized_order_bys_.empty()) {
     // Any explicit order by fields should be added as is.
     std::vector<OrderBy> result = explicit_order_bys_;
     std::set<FieldPath> fieldsNormalized;
@@ -127,8 +127,9 @@ const std::vector<OrderBy>& Query::normalized_order_bys() const {
       result.push_back(OrderBy(FieldPath::KeyFieldPath(), last_direction));
     }
 
-    return result;
-  });
+    memoized_normalized_order_bys_ = std::move(result);
+  }
+  return memoized_normalized_order_bys_;
 }
 
 LimitType Query::limit_type() const {
@@ -297,16 +298,23 @@ std::string Query::ToString() const {
 }
 
 const Target& Query::ToTarget() const& {
-  return memoized_target_->memoize(
-      [&]() { return ToTarget(normalized_order_bys()); });
+  if (memoized_target == nullptr) {
+    memoized_target = ToTarget(normalized_order_bys());
+  }
+
+  return *memoized_target;
 }
 
 const Target& Query::ToAggregateTarget() const& {
-  return memoized_aggregate_target_->memoize(
-      [&]() { return ToTarget(explicit_order_bys_); });
+  if (memoized_aggregate_target == nullptr) {
+    memoized_aggregate_target = ToTarget(explicit_order_bys_);
+  }
+
+  return *memoized_aggregate_target;
 }
 
-Target Query::ToTarget(const std::vector<OrderBy>& order_bys) const {
+const std::shared_ptr<const Target> Query::ToTarget(
+    const std::vector<OrderBy>& order_bys) const& {
   if (limit_type_ == LimitType::Last) {
     // Flip the orderBy directions since we want the last results
     std::vector<OrderBy> new_order_bys;
@@ -327,11 +335,13 @@ Target Query::ToTarget(const std::vector<OrderBy>& order_bys) const {
                                 start_at_->position(), start_at_->inclusive())}
                           : absl::nullopt;
 
-    return Target(path(), collection_group(), filters(), new_order_bys, limit_,
+    Target target(path(), collection_group(), filters(), new_order_bys, limit_,
                   new_start_at, new_end_at);
+    return std::make_shared<Target>(std::move(target));
   } else {
-    return Target(path(), collection_group(), filters(), order_bys, limit_,
+    Target target(path(), collection_group(), filters(), order_bys, limit_,
                   start_at(), end_at());
+    return std::make_shared<Target>(std::move(target));
   }
 }
 

Firestore/core/src/core/query.h

@@ -31,7 +31,6 @@
 #include "Firestore/core/src/core/target.h"
 #include "Firestore/core/src/model/model_fwd.h"
 #include "Firestore/core/src/model/resource_path.h"
-#include "Firestore/core/src/util/thread_safe_memoizer.h"
 
 namespace firebase {
 namespace firestore {
@@ -281,34 +280,25 @@ class Query {
   // sort at the end.
   std::vector<OrderBy> explicit_order_bys_;
 
+  // The memoized list of sort orders.
+  mutable std::vector<OrderBy> memoized_normalized_order_bys_;
+
   int32_t limit_ = Target::kNoLimit;
   LimitType limit_type_ = LimitType::None;
 
   absl::optional<Bound> start_at_;
   absl::optional<Bound> end_at_;
 
-  Target ToTarget(const std::vector<OrderBy>& order_bys) const;
-
-  // For properties below, use a `std::shared_ptr<ThreadSafeMemoizer>` rather
-  // than using `ThreadSafeMemoizer` directly so that this class is copyable
-  // (`ThreadSafeMemoizer` is not copyable because of its `std::once_flag`
-  // member variable, which is not copyable).
-
-  // The memoized list of sort orders.
-  mutable std::shared_ptr<util::ThreadSafeMemoizer<std::vector<OrderBy>>>
-      memoized_normalized_order_bys_{
-          std::make_shared<util::ThreadSafeMemoizer<std::vector<OrderBy>>>()};
-
   // The corresponding Target of this Query instance.
-  mutable std::shared_ptr<util::ThreadSafeMemoizer<Target>> memoized_target_{
-      std::make_shared<util::ThreadSafeMemoizer<Target>>()};
+  mutable std::shared_ptr<const Target> memoized_target;
 
   // The corresponding aggregate Target of this Query instance. Unlike targets
   // for non-aggregate queries, aggregate query targets do not contain
   // normalized order-bys, they only contain explicit order-bys.
-  mutable std::shared_ptr<util::ThreadSafeMemoizer<Target>>
-      memoized_aggregate_target_{
-          std::make_shared<util::ThreadSafeMemoizer<Target>>()};
+  mutable std::shared_ptr<const Target> memoized_aggregate_target;
+
+  const std::shared_ptr<const Target> ToTarget(
+      const std::vector<OrderBy>& order_bys) const&;
 };
 
 bool operator==(const Query& lhs, const Query& rhs);