Fix a crash that happens when Firestore is being terminated when the last shared pointer to it is in a listener (#7421)

If a user calls terminate and immediately disposes their reference to Firestore, it's possible that while termination is in process, the last remaining reference (more precisely, shared pointer) to Firestore is in a listener. When that listener is destroyed as part of the termination, it leads to `api::Firestore` being destroyed. Importantly, termination happens on the worker queue. The destructor of `api::Firestore` calls `Dispose` which presumes it's not called from the worker queue and tries to enqueue work, leading to a failing assertion and a crash.

The solution is simply to remove the sequential order checks when the queue enters/is in restricted mode. There is a legitimate case where the Firestore destructor can run on the worker queue, as the original issue shows. The complexity of #7412 also indicates that a simpler solution is preferable.

Fixes #6909.

Author: var-const

Firestore/CHANGELOG.md

@@ -1,3 +1,7 @@
+# Unreleased
+- [fixed] Fixed a crash that would happen when the app is being deleted and
+  immediately disposed of and there's an active listener (#6909).
+
 # v7.5.0
 - [changed] A write to a document that contains FieldValue transforms is no
   longer split up into two separate operations. This reduces the number of

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

@@ -1441,6 +1441,26 @@ - (void)testAppDeleteLeadsToFirestoreTermination {
   XCTAssertTrue(firestore.wrapped->client()->is_terminated());
 }
 
+// Ensures b/172958106 doesn't regress.
+- (void)testDeleteAppWorksWhenLastReferenceToFirestoreIsInListener {
+  FIRApp *app = testutil::AppForUnitTesting(util::MakeString([FSTIntegrationTestCase projectID]));
+  FIRFirestore *firestore = [FIRFirestore firestoreForApp:app];
+
+  FIRDocumentReference *doc = [firestore documentWithPath:@"abc/123"];
+  // Make sure there is a listener.
+  [doc addSnapshotListener:^(FIRDocumentSnapshot *, NSError *){
+  }];
+
+  XCTestExpectation *expectation = [self expectationWithDescription:@"App is deleted"];
+  [app deleteApp:^(BOOL) {
+    [expectation fulfill];
+  }];
+  // Let go of the last app reference.
+  app = nil;
+
+  [self awaitExpectations];
+}
+
 - (void)testTerminateCanBeCalledMultipleTimes {
   FIRApp *app = testutil::AppForUnitTesting(util::MakeString([FSTIntegrationTestCase projectID]));
   FIRFirestore *firestore = [FIRFirestore firestoreForApp:app];

Firestore/core/src/util/async_queue.cc

@@ -45,7 +45,6 @@ AsyncQueue::~AsyncQueue() {
 
 void AsyncQueue::EnterRestrictedMode() {
   std::lock_guard<std::mutex> lock(mutex_);
-  VerifySequentialOrder();
   if (mode_ == Mode::kDisposed) return;
 
   mode_ = Mode::kRestricted;
@@ -97,9 +96,7 @@ bool AsyncQueue::Enqueue(const Operation& operation) {
 }
 
 bool AsyncQueue::EnqueueEvenWhileRestricted(const Operation& operation) {
-  // Still guarding the lock to ensure sequential scheduling.
   std::lock_guard<std::mutex> lock(mutex_);
-  VerifySequentialOrder();
   if (mode_ == Mode::kDisposed) return false;
 
   executor_->Execute(Wrap(operation));