Merge d0681c9 into 26075af

firebase · Aug 3, 2022 · d329277 · d329277
2 parents 26075af + d0681c9
commit d329277
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 89 deletions.
diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md
@@ -1,3 +1,7 @@
+# Unreleased
+- [fixed] Fixed an intermittent crash if `ListenerRegistration::Remove()` was
+  invoked concurrently (#10065).
+
 # 9.4.0
 - [fixed] Fixed a crash during app start (#9985, #10018).
 

diff --git a/Firestore/core/src/api/query_listener_registration.cc b/Firestore/core/src/api/query_listener_registration.cc
@@ -22,6 +22,7 @@
 #include "Firestore/core/src/core/firestore_client.h"
 #include "Firestore/core/src/core/query_listener.h"
 #include "Firestore/core/src/core/view_snapshot.h"
+#include "absl/synchronization/mutex.h"
 
 namespace firebase {
 namespace firestore {
@@ -38,6 +39,8 @@ QueryListenerRegistration::QueryListenerRegistration(
 }
 
 void QueryListenerRegistration::Remove() {
+  absl::MutexLock lock(&mutex_);
+
   auto async_listener = async_listener_.lock();
   if (async_listener) {
     async_listener->Mute();

diff --git a/Firestore/core/src/api/query_listener_registration.h b/Firestore/core/src/api/query_listener_registration.h
@@ -21,6 +21,8 @@
 
 #include "Firestore/core/src/api/listener_registration.h"
 #include "Firestore/core/src/core/core_fwd.h"
+#include "absl/base/thread_annotations.h"
+#include "absl/synchronization/mutex.h"
 
 namespace firebase {
 namespace firestore {
@@ -44,14 +46,17 @@ class QueryListenerRegistration : public ListenerRegistration {
   void Remove() override;
 
  private:
+  absl::Mutex mutex_;
+
   /** The client that was used to register this listen. */
-  std::shared_ptr<core::FirestoreClient> client_;
+  std::shared_ptr<core::FirestoreClient> client_ ABSL_GUARDED_BY(mutex_);
 
   /** The async listener that is used to mute events synchronously. */
-  std::weak_ptr<core::AsyncEventListener<core::ViewSnapshot>> async_listener_;
+  std::weak_ptr<core::AsyncEventListener<core::ViewSnapshot>> async_listener_
+      ABSL_GUARDED_BY(mutex_);
 
   /** The internal QueryListener that can be used to unlisten the query. */
-  std::weak_ptr<core::QueryListener> query_listener_;
+  std::weak_ptr<core::QueryListener> query_listener_ ABSL_GUARDED_BY(mutex_);
 };
 
 }  // namespace api

diff --git a/scripts/check.sh b/scripts/check.sh
@@ -291,7 +291,6 @@ python --version
 "${top_dir}/scripts/check_copyright.sh"
 "${top_dir}/scripts/check_test_inclusion.py"
 "${top_dir}/scripts/check_imports.swift"
-"${top_dir}/scripts/check_firestore_core_api_absl.py"
 
 # Google C++ style
 lint_cmd=("${top_dir}/scripts/check_lint.py")

diff --git a/scripts/check_firestore_core_api_absl.py b/scripts/check_firestore_core_api_absl.py