Avoid blocking access to the AsyncQueue during Dispose (#5635)

Author: wilhuff

Firestore/core/src/util/async_queue.cc

@@ -52,9 +52,11 @@ void AsyncQueue::EnterRestrictedMode() {
 }
 
 void AsyncQueue::Dispose() {
-  std::lock_guard<std::mutex> lock(mutex_);
+  {
+    std::lock_guard<std::mutex> lock(mutex_);
+    mode_ = Mode::kDisposed;
+  }
 
-  mode_ = Mode::kDisposed;
   executor_->Dispose();
 }
 

Firestore/core/test/unit/util/async_queue_test.cc

@@ -224,7 +224,7 @@ TEST_P(AsyncQueueTest, CanScheduleOprationsWithRespectsToShutdownState) {
   EXPECT_EQ(steps, "124");
 }
 
-TEST_P(AsyncQueueTest, RestrictedModeBlocksEnqueue) {
+TEST_P(AsyncQueueTest, RestrictedModePreventsEnqueue) {
   ASSERT_TRUE(queue->Enqueue([&] {}));
   ASSERT_TRUE(queue->EnqueueEvenWhileRestricted([&] {}));
 
@@ -233,7 +233,7 @@ TEST_P(AsyncQueueTest, RestrictedModeBlocksEnqueue) {
   ASSERT_TRUE(queue->EnqueueEvenWhileRestricted([&] {}));
 }
 
-TEST_P(AsyncQueueTest, DisposeBlocksAllEnqueues) {
+TEST_P(AsyncQueueTest, DisposePreventsAllEnqueues) {
   ASSERT_TRUE(queue->Enqueue([&] {}));
   ASSERT_TRUE(queue->EnqueueEvenWhileRestricted([&] {}));
 
@@ -242,6 +242,42 @@ TEST_P(AsyncQueueTest, DisposeBlocksAllEnqueues) {
   ASSERT_FALSE(queue->EnqueueEvenWhileRestricted([&] {}));
 }
 
+TEST_P(AsyncQueueTest, DisposeDoesNotBlockEnqueueWhileWaiting) {
+  // Start a task that will block the queue. AsyncQueue::Dispose will block
+  // until this completes.
+  Expectation blocking_started;
+  Expectation blocking_complete;
+  queue->Enqueue([&] {
+    blocking_started.Fulfill();
+    Await(blocking_complete);
+  });
+
+  // Kick off Dispose--this will block while the task above is still running.
+  Await(blocking_started);
+  Expectation dispose_started;
+  Expectation dispose_complete;
+  Async([&] {
+    dispose_started.Fulfill();
+    queue->Dispose();
+    dispose_complete.Fulfill();
+  });
+
+  // Finally, try to enqueue while Dispose is blocked waiting for the first
+  // task to complete. This should not block.
+  Expectation enqueue_completed;
+  Expectation post_dispose;
+  Async([&] {
+    Await(dispose_started);
+    bool enqueued = queue->Enqueue(post_dispose.AsCallback());
+    ASSERT_FALSE(enqueued);
+    enqueue_completed.Fulfill();
+  });
+
+  Await(enqueue_completed);
+  blocking_complete.Fulfill();
+  Await(dispose_complete);
+}
+
 }  // namespace util
 }  // namespace firestore
 }  // namespace firebase