Fix: whereField creates a memory leak

Firestore/CHANGELOG.md

@@ -1,3 +1,6 @@
+# Unreleased
+- [fixed] Fix the memory leak while using `whereField` (#12613)
+
 # 10.25.0
 - [fixed] Allow blob of data with zero length. (#11773, #12620)
 - [changed] Passing a non-nil value to the `@DocumentID` property wrapper's

Firestore/Example/Firestore.xcodeproj/project.pbxproj

@@ -73,7 +73,6 @@
 		095A878BB33211AB52BFAD9F /* leveldb_document_overlay_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AE89CFF09C6804573841397F /* leveldb_document_overlay_cache_test.cc */; };
 		0963F6D7B0F9AE1E24B82866 /* path_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 403DBF6EFB541DFD01582AA3 /* path_test.cc */; };
 		096BA3A3703AC1491F281618 /* index.pb.cc in Sources */ = {isa = PBXBuildFile; fileRef = 395E8B07639E69290A929695 /* index.pb.cc */; };
-		09B83B26E47B6F6668DF54B8 /* thread_safe_memoizer_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1A8141230C7E3986EACEF0B6 /* thread_safe_memoizer_test.cc */; };
 		09BE8C01EC33D1FD82262D5D /* aggregate_query_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AF924C79F49F793992A84879 /* aggregate_query_test.cc */; };
 		0A4E1B5E3E853763AE6ED7AE /* grpc_stream_tester.cc in Sources */ = {isa = PBXBuildFile; fileRef = 87553338E42B8ECA05BA987E /* grpc_stream_tester.cc */; };
 		0A52B47C43B7602EE64F53A7 /* cc_compilation_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1B342370EAE3AA02393E33EB /* cc_compilation_test.cc */; };
@@ -231,7 +230,6 @@
 		205601D1C6A40A4DD3BBAA04 /* target_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 526D755F65AC676234F57125 /* target_test.cc */; };
 		20814A477D00EA11D0E76631 /* FIRDocumentSnapshotTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E04B202154AA00B64F25 /* FIRDocumentSnapshotTests.mm */; };
 		20A26E9D0336F7F32A098D05 /* Pods_Firestore_IntegrationTests_tvOS.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 2220F583583EFC28DE792ABE /* Pods_Firestore_IntegrationTests_tvOS.framework */; };
-		20A93AC59CD5A7AC41F10412 /* thread_safe_memoizer_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1A8141230C7E3986EACEF0B6 /* thread_safe_memoizer_test.cc */; };
 		211A60ECA3976D27C0BF59BB /* md5_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 3D050936A2D52257FD17FB6E /* md5_test.cc */; };
 		21836C4D9D48F962E7A3A244 /* ordered_code_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB380D03201BC6E400D97691 /* ordered_code_test.cc */; };
 		21A2A881F71CB825299DF06E /* hard_assert_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 444B7AB3F5A2929070CB1363 /* hard_assert_test.cc */; };
@@ -505,7 +503,6 @@
 		50454F81EC4584D4EB5F5ED5 /* serializer_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 61F72C5520BC48FD001A68CB /* serializer_test.cc */; };
 		50B749CA98365368AE34B71C /* filter_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = F02F734F272C3C70D1307076 /* filter_test.cc */; };
 		50C852E08626CFA7DC889EEA /* field_index_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = BF76A8DA34B5B67B4DD74666 /* field_index_test.cc */; };
-		51018EA27CF914DD1CC79CB3 /* thread_safe_memoizer_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1A8141230C7E3986EACEF0B6 /* thread_safe_memoizer_test.cc */; };
 		513D34C9964E8C60C5C2EE1C /* leveldb_bundle_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 8E9CD82E60893DDD7757B798 /* leveldb_bundle_cache_test.cc */; };
 		5150E9F256E6E82D6F3CB3F1 /* bundle_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = F7FC06E0A47D393DE1759AE1 /* bundle_cache_test.cc */; };
 		518BF03D57FBAD7C632D18F8 /* FIRQueryUnitTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = FF73B39D04D1760190E6B84A /* FIRQueryUnitTests.mm */; };
@@ -669,7 +666,6 @@
 		5B4391097A6DF86EC3801DEE /* string_win_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 79507DF8378D3C42F5B36268 /* string_win_test.cc */; };
 		5B62003FEA9A3818FDF4E2DD /* document_key_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = B6152AD5202A5385000E5744 /* document_key_test.cc */; };
 		5B89B1BA0AD400D9BF581420 /* listen_spec_test.json in Resources */ = {isa = PBXBuildFile; fileRef = 54DA12A01F315EE100DD57A1 /* listen_spec_test.json */; };
-		5BB33F0BC7960D26062B07D3 /* thread_safe_memoizer_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1A8141230C7E3986EACEF0B6 /* thread_safe_memoizer_test.cc */; };
 		5BC8406FD842B2FC2C200B2F /* stream_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 5B5414D28802BC76FDADABD6 /* stream_test.cc */; };
 		5BE49546D57C43DDFCDB6FBD /* to_string_apple_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = B68B1E002213A764008977EF /* to_string_apple_test.mm */; };
 		5C9B5696644675636A052018 /* token_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = A082AFDD981B07B5AD78FDE8 /* token_test.cc */; };
@@ -797,7 +793,6 @@
 		6D7F70938662E8CA334F11C2 /* target_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = B5C37696557C81A6C2B7271A /* target_cache_test.cc */; };
 		6DBB3DB3FD6B4981B7F26A55 /* FIRQuerySnapshotTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E04F202154AA00B64F25 /* FIRQuerySnapshotTests.mm */; };
 		6DCA8E54E652B78EFF3EEDAC /* XCTestCase+Await.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E0372021401E00B64F25 /* XCTestCase+Await.mm */; };
-		6DFD49CCE2281CE243FEBB63 /* thread_safe_memoizer_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1A8141230C7E3986EACEF0B6 /* thread_safe_memoizer_test.cc */; };
 		6E10507432E1D7AE658D16BD /* FSTSpecTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E03020213FFC00B64F25 /* FSTSpecTests.mm */; };
 		6E4854B19B120C6F0F8192CC /* FSTAPIHelpers.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E04E202154AA00B64F25 /* FSTAPIHelpers.mm */; };
 		6E59498D20F55BA800ECD9A5 /* FuzzingResources in Resources */ = {isa = PBXBuildFile; fileRef = 6ED6DEA120F5502700FC6076 /* FuzzingResources */; };
@@ -1178,7 +1173,6 @@
 		B2554A2BA211D10823646DBE /* Validation_BloomFilterTest_MD5_500_01_bloom_filter_proto.json in Resources */ = {isa = PBXBuildFile; fileRef = 4BD051DBE754950FEAC7A446 /* Validation_BloomFilterTest_MD5_500_01_bloom_filter_proto.json */; };
 		B28ACC69EB1F232AE612E77B /* async_testing.cc in Sources */ = {isa = PBXBuildFile; fileRef = 872C92ABD71B12784A1C5520 /* async_testing.cc */; };
 		B2A9965ED0114E39A911FD09 /* Validation_BloomFilterTest_MD5_5000_1_bloom_filter_proto.json in Resources */ = {isa = PBXBuildFile; fileRef = 4375BDCDBCA9938C7F086730 /* Validation_BloomFilterTest_MD5_5000_1_bloom_filter_proto.json */; };
-		B31B5E0D4EA72C5916CC71F5 /* thread_safe_memoizer_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1A8141230C7E3986EACEF0B6 /* thread_safe_memoizer_test.cc */; };
 		B371628DA91E80B64AE53085 /* FIRFieldPathTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E04C202154AA00B64F25 /* FIRFieldPathTests.mm */; };
 		B384E0F90D4CCC15C88CAF30 /* target_index_matcher_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 63136A2371C0C013EC7A540C /* target_index_matcher_test.cc */; };
 		B3A309CCF5D75A555C7196E1 /* path_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 403DBF6EFB541DFD01582AA3 /* path_test.cc */; };
@@ -1693,7 +1687,6 @@
 		132E32997D781B896672D30A /* reference_set_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = reference_set_test.cc; sourceTree = "<group>"; };
 		166CE73C03AB4366AAC5201C /* leveldb_index_manager_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = leveldb_index_manager_test.cc; sourceTree = "<group>"; };
 		1A7D48A017ECB54FD381D126 /* Validation_BloomFilterTest_MD5_5000_1_membership_test_result.json */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.json; name = Validation_BloomFilterTest_MD5_5000_1_membership_test_result.json; path = bloom_filter_golden_test_data/Validation_BloomFilterTest_MD5_5000_1_membership_test_result.json; sourceTree = "<group>"; };
-		1A8141230C7E3986EACEF0B6 /* thread_safe_memoizer_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = thread_safe_memoizer_test.cc; sourceTree = "<group>"; };
 		1B342370EAE3AA02393E33EB /* cc_compilation_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; name = cc_compilation_test.cc; path = api/cc_compilation_test.cc; sourceTree = "<group>"; };
 		1B9F95EC29FAD3F100EEC075 /* FIRAggregateQueryUnitTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FIRAggregateQueryUnitTests.mm; sourceTree = "<group>"; };
 		1C01D8CE367C56BB2624E299 /* index.pb.h */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.c.h; name = index.pb.h; path = admin/index.pb.h; sourceTree = "<group>"; };
@@ -2384,7 +2377,6 @@
 				79507DF8378D3C42F5B36268 /* string_win_test.cc */,
 				899FC22684B0F7BEEAE13527 /* task_test.cc */,
 				A002425BC4FC4E805F4175B6 /* testing_hooks_test.cc */,
-				1A8141230C7E3986EACEF0B6 /* thread_safe_memoizer_test.cc */,
 				B68B1E002213A764008977EF /* to_string_apple_test.mm */,
 				B696858D2214B53900271095 /* to_string_test.cc */,
 			);
@@ -4292,7 +4284,6 @@
 				88929ED628DA8DD9592974ED /* task_test.cc in Sources */,
 				9B2C6A48A4DBD36080932B4E /* testing_hooks_test.cc in Sources */,
 				32A95242C56A1A230231DB6A /* testutil.cc in Sources */,
-				51018EA27CF914DD1CC79CB3 /* thread_safe_memoizer_test.cc in Sources */,
 				5497CB78229DECDE000FB92F /* time_testing.cc in Sources */,
 				ACC9369843F5ED3BD2284078 /* timestamp_test.cc in Sources */,
 				2AAEABFD550255271E3BAC91 /* to_string_apple_test.mm in Sources */,
@@ -4510,7 +4501,6 @@
 				67CF9FAA890307780731E1DA /* task_test.cc in Sources */,
 				24B75C63BDCD5551B2F69901 /* testing_hooks_test.cc in Sources */,
 				8388418F43042605FB9BFB92 /* testutil.cc in Sources */,
-				5BB33F0BC7960D26062B07D3 /* thread_safe_memoizer_test.cc in Sources */,
 				5497CB79229DECDE000FB92F /* time_testing.cc in Sources */,
 				26CB3D7C871BC56456C6021E /* timestamp_test.cc in Sources */,
 				5BE49546D57C43DDFCDB6FBD /* to_string_apple_test.mm in Sources */,
@@ -4751,7 +4741,6 @@
 				76A5447D76F060E996555109 /* task_test.cc in Sources */,
 				D0DA42DC66C4FE508A63B269 /* testing_hooks_test.cc in Sources */,
 				409C0F2BFC2E1BECFFAC4D32 /* testutil.cc in Sources */,
-				B31B5E0D4EA72C5916CC71F5 /* thread_safe_memoizer_test.cc in Sources */,
 				6300709ECDE8E0B5A8645F8D /* time_testing.cc in Sources */,
 				0CEE93636BA4852D3C5EC428 /* timestamp_test.cc in Sources */,
 				95DCD082374F871A86EF905F /* to_string_apple_test.mm in Sources */,
@@ -4992,7 +4981,6 @@
 				93C8F772F4DC5A985FA3D815 /* task_test.cc in Sources */,
 				F6738D3B72352BBEFB87172C /* testing_hooks_test.cc in Sources */,
 				A17DBC8F24127DA8A381F865 /* testutil.cc in Sources */,
-				09B83B26E47B6F6668DF54B8 /* thread_safe_memoizer_test.cc in Sources */,
 				A25FF76DEF542E01A2DF3B0E /* time_testing.cc in Sources */,
 				1E42CD0F60EB22A5D0C86D1F /* timestamp_test.cc in Sources */,
 				F9705E595FC3818F13F6375A /* to_string_apple_test.mm in Sources */,
@@ -5220,7 +5208,6 @@
 				662793139A36E5CFC935B949 /* task_test.cc in Sources */,
 				F184E5367DF3CA158EDE8532 /* testing_hooks_test.cc in Sources */,
 				54A0352A20A3B3BD003E0143 /* testutil.cc in Sources */,
-				20A93AC59CD5A7AC41F10412 /* thread_safe_memoizer_test.cc in Sources */,
 				5497CB77229DECDE000FB92F /* time_testing.cc in Sources */,
 				ABF6506C201131F8005F2C74 /* timestamp_test.cc in Sources */,
 				B68B1E012213A765008977EF /* to_string_apple_test.mm in Sources */,
@@ -5480,7 +5467,6 @@
 				C57B15CADD8C3E806B154C19 /* task_test.cc in Sources */,
 				5360D52DCAD1069B1E4B0B9D /* testing_hooks_test.cc in Sources */,
 				CA989C0E6020C372A62B7062 /* testutil.cc in Sources */,
-				6DFD49CCE2281CE243FEBB63 /* thread_safe_memoizer_test.cc in Sources */,
 				2D220B9ABFA36CD7AC43D0A7 /* time_testing.cc in Sources */,
 				D91D86B29B86A60C05879A48 /* timestamp_test.cc in Sources */,
 				60260A06871DCB1A5F3448D3 /* to_string_apple_test.mm in Sources */,

Firestore/core/src/api/query_core.cc

@@ -340,7 +340,11 @@ void Query::ValidateNewFieldFilter(const core::Query& query,
 
 void Query::ValidateNewFilter(const Filter& filter) const {
   core::Query test_query(query_);
-  for (const auto& field_filter : filter.GetFlattenedFilters()) {
+  // Don't deference filter.GetFlattenedFilters() directly. if the filter is a
+  // field filter, calling for (const FieldFilter& subFilter :
+  // *filter.GetFlattenedFilters()) will deference a temporary object.
+  auto flattened_filters_ptr = filter.GetFlattenedFilters();
+  for (const FieldFilter& field_filter : *flattened_filters_ptr) {
     ValidateNewFieldFilter(test_query, field_filter);
     test_query = test_query.AddingFilter(field_filter);
   }

Firestore/core/src/core/composite_filter.cc

@@ -17,6 +17,7 @@
 #include "Firestore/core/src/core/composite_filter.h"
 
 #include <algorithm>
+#include <iterator>
 #include <utility>
 
 #include "Firestore/core/src/core/field_filter.h"
@@ -58,6 +59,13 @@ CompositeFilter::CompositeFilter(std::shared_ptr<const Filter::Rep>&& rep)
 
 CompositeFilter::Rep::Rep(std::vector<Filter>&& filters, Operator op)
     : filters_(std::move(filters)), op_(op) {
+  std::vector<FieldFilter> flattened_filters;
+  for (const auto& filter : filters_) {
+    auto ptr = filter.GetFlattenedFilters();
+    std::copy(ptr->begin(), ptr->end(), std::back_inserter(flattened_filters));
+  }
+  flattened_filters_ = std::make_shared<const std::vector<FieldFilter>>(
+      std::move(flattened_filters));
 }
 
 CompositeFilter::CompositeFilter(const Filter& other) : Filter(other) {
@@ -133,24 +141,17 @@ CompositeFilter CompositeFilter::WithAddedFilters(
 
 const FieldFilter* CompositeFilter::Rep::FindFirstMatchingFilter(
     const CheckFunction& condition) const {
-  for (const auto& field_filter : GetFlattenedFilters()) {
+  for (const auto& field_filter : *GetFlattenedFilters()) {
     if (condition(field_filter)) {
       return &field_filter;
     }
   }
   return nullptr;
 }
 
-const std::vector<FieldFilter>& CompositeFilter::Rep::GetFlattenedFilters()
-    const {
-  return memoized_flattened_filters_->memoize([&]() {
-    std::vector<FieldFilter> flattened_filters;
-    for (const auto& filter : filters())
-      std::copy(filter.GetFlattenedFilters().begin(),
-                filter.GetFlattenedFilters().end(),
-                std::back_inserter(flattened_filters));
-    return flattened_filters;
-  });
+std::shared_ptr<const std::vector<FieldFilter>>
+CompositeFilter::Rep::GetFlattenedFilters() const {
+  return flattened_filters_;
 }
 
 }  // namespace core

Firestore/core/src/core/composite_filter.h

@@ -138,7 +138,8 @@ class CompositeFilter : public Filter {
       return filters_.empty();
     }
 
-    const std::vector<FieldFilter>& GetFlattenedFilters() const override;
+    std::shared_ptr<const std::vector<FieldFilter>> GetFlattenedFilters()
+        const override;
 
     std::vector<Filter> GetFilters() const override {
       return filters();
@@ -158,6 +159,12 @@ class CompositeFilter : public Filter {
     /** The type of and/or operator in the composite filter. */
     Operator op_;
 
+    /**
+     * A list of all field filters that are contained within this composite
+     * filter.
+     */
+    std::shared_ptr<const std::vector<FieldFilter>> flattened_filters_;
+
     friend class CompositeFilter;
   };
 

Firestore/core/src/core/field_filter.cc

@@ -17,6 +17,7 @@
 #include "Firestore/core/src/core/field_filter.h"
 
 #include <utility>
+#include <vector>
 
 #include "Firestore/core/src/core/array_contains_any_filter.h"
 #include "Firestore/core/src/core/array_contains_filter.h"
@@ -122,12 +123,12 @@ FieldFilter::FieldFilter(std::shared_ptr<const Filter::Rep> rep)
     : Filter(std::move(rep)) {
 }
 
-const std::vector<FieldFilter>& FieldFilter::Rep::GetFlattenedFilters() const {
+std::shared_ptr<const std::vector<FieldFilter>>
+FieldFilter::Rep::GetFlattenedFilters() const {
   // This is already a field filter, so we return a vector of size one.
-  return memoized_flattened_filters_->memoize([&]() {
-    return std::vector<FieldFilter>{
-        FieldFilter(std::make_shared<const Rep>(*this))};
-  });
+  return std::make_shared<const std::vector<FieldFilter>>(
+      std::vector<FieldFilter>{
+          FieldFilter(std::make_shared<const Rep>(*this))});
 }
 
 std::vector<Filter> FieldFilter::Rep::GetFilters() const {

Firestore/core/src/core/field_filter.h

@@ -117,7 +117,8 @@ class FieldFilter : public Filter {
       return false;
     }
 
-    const std::vector<FieldFilter>& GetFlattenedFilters() const override;
+    std::shared_ptr<const std::vector<FieldFilter>> GetFlattenedFilters()
+        const override;
 
     std::vector<Filter> GetFilters() const override;
 

Firestore/core/src/core/filter.cc

@@ -18,9 +18,6 @@
 
 #include <ostream>
 
-#include "Firestore/core/src/core/field_filter.h"
-#include "Firestore/core/src/util/thread_safe_memoizer.h"
-
 namespace firebase {
 namespace firestore {
 namespace core {
@@ -35,12 +32,6 @@ std::ostream& operator<<(std::ostream& os, const Filter& filter) {
   return os << filter.ToString();
 }
 
-Filter::Rep::Rep()
-    : memoized_flattened_filters_(
-          std::make_shared<
-              util::ThreadSafeMemoizer<std::vector<FieldFilter>>>()) {
-}
-
 }  // namespace core
 }  // namespace firestore
 }  // namespace firebase

Firestore/core/src/core/filter.h

@@ -23,7 +23,6 @@
 #include <vector>
 
 #include "Firestore/core/src/model/model_fwd.h"
-#include "Firestore/core/src/util/thread_safe_memoizer.h"
 
 namespace firebase {
 namespace firestore {
@@ -98,7 +97,7 @@ class Filter {
   /**
    * Returns a list of all field filters that are contained within this filter.
    */
-  const std::vector<FieldFilter>& GetFlattenedFilters() const {
+  std::shared_ptr<const std::vector<FieldFilter>> GetFlattenedFilters() const {
     return rep_->GetFlattenedFilters();
   }
 
@@ -114,8 +113,6 @@ class Filter {
  protected:
   class Rep {
    public:
-    Rep();
-
     virtual ~Rep() = default;
 
     virtual Type type() const {
@@ -147,21 +144,10 @@ class Filter {
 
     virtual bool IsEmpty() const = 0;
 
-    virtual const std::vector<FieldFilter>& GetFlattenedFilters() const = 0;
+    virtual std::shared_ptr<const std::vector<FieldFilter>>
+    GetFlattenedFilters() const = 0;
 
     virtual std::vector<Filter> GetFilters() const = 0;
-
-    /**
-     * Memoized list of all field filters that can be found by
-     * traversing the tree of filters contained in this composite filter.
-     *
-     * 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).
-     */
-    mutable std::shared_ptr<util::ThreadSafeMemoizer<std::vector<FieldFilter>>>
-        memoized_flattened_filters_;
   };
 
   explicit Filter(std::shared_ptr<const Rep>&& rep) : rep_(rep) {