Firestore C++: make FSTDispatchQueue delegate to C++ implementation (#1240)

FSTDispatchQueue now doesn't contain any logic of its own and instead
just passes through all method calls to AsyncQueue (backed by an
ExecutorLibdispatch).

Firestore/Example/Tests/Util/FSTDispatchQueueTests.mm

@@ -66,9 +66,10 @@ static const FSTTimerID timerID3 = FSTTimerIDWriteStreamConnectionBackoff;
   XCTAssertNotNil(caught);
 
   XCTAssertEqualObjects(caught.name, NSInternalInconsistencyException);
-  XCTAssertTrue(
-      [caught.reason hasPrefix:@"FIRESTORE INTERNAL ASSERTION FAILED: "
-                               @"dispatchAsync called when we are already running on target"]);
+  XCTAssertTrue([caught.reason
+      hasPrefix:
+          @"FIRESTORE INTERNAL ASSERTION FAILED: "
+          @"Enqueue methods cannot be called when we are already running on target executor"]);
 }
 
 - (void)testDispatchAsyncAllowingSameQueueActuallyAllowsSameQueue {
@@ -133,9 +134,10 @@ static const FSTTimerID timerID3 = FSTTimerIDWriteStreamConnectionBackoff;
   XCTAssertNotNil(caught);
 
   XCTAssertEqualObjects(caught.name, NSInternalInconsistencyException);
-  XCTAssertTrue(
-      [caught.reason hasPrefix:@"FIRESTORE INTERNAL ASSERTION FAILED: "
-                               @"dispatchSync called when we are already running on target"]);
+  XCTAssertTrue([caught.reason
+      hasPrefix:
+          @"FIRESTORE INTERNAL ASSERTION FAILED: "
+          @"Enqueue methods cannot be called when we are already running on target executor"]);
 }
 
 - (void)testVerifyIsCurrentQueueActuallyRequiresCurrentQueue {
@@ -150,7 +152,8 @@ static const FSTTimerID timerID3 = FSTTimerIDWriteStreamConnectionBackoff;
   }
   XCTAssertNotNil(caught);
   XCTAssertTrue([caught.reason hasPrefix:@"FIRESTORE INTERNAL ASSERTION FAILED: "
-                                         @"We are running on the wrong dispatch queue"]);
+                                         @"Expected to be called by the executor "
+                                         @"associated with this queue"]);
 }
 
 - (void)testVerifyIsCurrentQueueRequiresOperationIsInProgress {
@@ -165,7 +168,7 @@ static const FSTTimerID timerID3 = FSTTimerIDWriteStreamConnectionBackoff;
   XCTAssertNotNil(caught);
   XCTAssertTrue(
       [caught.reason hasPrefix:@"FIRESTORE INTERNAL ASSERTION FAILED: "
-                               @"verifyIsCurrentQueue called outside enterCheckedOperation"]);
+                               @"VerifyIsCurrentQueue called when no operation is executing"]);
 }
 
 - (void)testVerifyIsCurrentQueueWorksWithOperationIsInProgress {
@@ -194,9 +197,10 @@ static const FSTTimerID timerID3 = FSTTimerIDWriteStreamConnectionBackoff;
   }];
   XCTAssertNil(problem);
   XCTAssertNotNil(caught);
-  XCTAssertTrue([caught.reason
-      hasPrefix:@"FIRESTORE INTERNAL ASSERTION FAILED: "
-                @"enterCheckedOperation may not be called when an operation is in progress"]);
+  XCTAssertTrue(
+      [caught.reason hasPrefix:@"FIRESTORE INTERNAL ASSERTION FAILED: "
+                               @"ExecuteBlocking may not be called before the previous operation "
+                               @"finishes executing"]);
 }
 
 /**
@@ -217,8 +221,10 @@ static const FSTTimerID timerID3 = FSTTimerIDWriteStreamConnectionBackoff;
   _expectation = [self expectationWithDescription:@"Expected steps"];
   _expectedSteps = @[ @1, @2, @3, @4 ];
   [_queue dispatchAsync:[self blockForStep:1]];
-  [_queue dispatchAfterDelay:0.005 timerID:timerID1 block:[self blockForStep:4]];
-  [_queue dispatchAfterDelay:0.001 timerID:timerID2 block:[self blockForStep:3]];
+  [_queue dispatchAsync:^{
+    [_queue dispatchAfterDelay:0.005 timerID:timerID1 block:[self blockForStep:4]];
+    [_queue dispatchAfterDelay:0.001 timerID:timerID2 block:[self blockForStep:3]];
+  }];
   [_queue dispatchAsync:[self blockForStep:2]];
 
   [self awaitExpectations];
@@ -244,8 +250,10 @@ static const FSTTimerID timerID3 = FSTTimerIDWriteStreamConnectionBackoff;
 
 - (void)testCanManuallyDrainAllDelayedCallbacksForTesting {
   [_queue dispatchAsync:[self blockForStep:1]];
-  [_queue dispatchAfterDelay:20 timerID:timerID1 block:[self blockForStep:4]];
-  [_queue dispatchAfterDelay:10 timerID:timerID2 block:[self blockForStep:3]];
+  [_queue dispatchAsync:^{
+    [_queue dispatchAfterDelay:20 timerID:timerID1 block:[self blockForStep:4]];
+    [_queue dispatchAfterDelay:10 timerID:timerID2 block:[self blockForStep:3]];
+  }];
   [_queue dispatchAsync:[self blockForStep:2]];
 
   [_queue runDelayedCallbacksUntil:FSTTimerIDAll];
@@ -254,9 +262,11 @@ static const FSTTimerID timerID3 = FSTTimerIDWriteStreamConnectionBackoff;
 
 - (void)testCanManuallyDrainSpecificDelayedCallbacksForTesting {
   [_queue dispatchAsync:[self blockForStep:1]];
-  [_queue dispatchAfterDelay:20 timerID:timerID1 block:[self blockForStep:5]];
-  [_queue dispatchAfterDelay:10 timerID:timerID2 block:[self blockForStep:3]];
-  [_queue dispatchAfterDelay:15 timerID:timerID3 block:[self blockForStep:4]];
+  [_queue dispatchAsync:^{
+    [_queue dispatchAfterDelay:20 timerID:timerID1 block:[self blockForStep:5]];
+    [_queue dispatchAfterDelay:10 timerID:timerID2 block:[self blockForStep:3]];
+    [_queue dispatchAfterDelay:15 timerID:timerID3 block:[self blockForStep:4]];
+  }];
   [_queue dispatchAsync:[self blockForStep:2]];
 
   [_queue runDelayedCallbacksUntil:timerID3];

Firestore/Source/Util/FSTDispatchQueue.mm

@@ -16,154 +16,66 @@
 
 #import <Foundation/Foundation.h>
 
-#include <atomic>
+#include <memory>
+#include <utility>
 
 #import "Firestore/Source/Util/FSTAssert.h"
 #import "Firestore/Source/Util/FSTDispatchQueue.h"
 
-NS_ASSUME_NONNULL_BEGIN
-
-/**
- * removeDelayedCallback is used by FSTDelayedCallback and so we pre-declare it before the rest of
- * the FSTDispatchQueue private interface.
- */
-@interface FSTDispatchQueue ()
-- (void)removeDelayedCallback:(FSTDelayedCallback *)callback;
-@end
+#include "Firestore/core/src/firebase/firestore/util/async_queue.h"
+#include "Firestore/core/src/firebase/firestore/util/executor_libdispatch.h"
+#include "absl/memory/memory.h"
 
-#pragma mark - FSTDelayedCallback
-
-/**
- * Represents a callback scheduled to be run in the future on an FSTDispatchQueue.
- *
- * It is created via [FSTDelayedCallback createAndScheduleWithQueue].
- *
- * Supports cancellation (via cancel) and early execution (via skipDelay).
- */
-@interface FSTDelayedCallback ()
+using firebase::firestore::util::AsyncQueue;
+using firebase::firestore::util::DelayedOperation;
+using firebase::firestore::util::TimerId;
+using firebase::firestore::util::internal::Executor;
+using firebase::firestore::util::internal::ExecutorLibdispatch;
 
-@property(nonatomic, strong, readonly) FSTDispatchQueue *queue;
-@property(nonatomic, assign, readonly) FSTTimerID timerID;
-@property(nonatomic, assign, readonly) NSTimeInterval targetTime;
-@property(nonatomic, copy) void (^callback)();
-/** YES if the callback has been run or canceled. */
-@property(nonatomic, getter=isDone) BOOL done;
+NS_ASSUME_NONNULL_BEGIN
 
-/**
- * Creates and returns an FSTDelayedCallback that has been scheduled on the provided queue with the
- * provided delay.
- *
- * @param queue The FSTDispatchQueue to run the callback on.
- * @param timerID A FSTTimerID identifying the type of the delayed callback.
- * @param delay The delay before the callback should be scheduled.
- * @param callback The callback block to run.
- * @return The created FSTDelayedCallback instance.
- */
-+ (instancetype)createAndScheduleWithQueue:(FSTDispatchQueue *)queue
-                                   timerID:(FSTTimerID)timerID
-                                     delay:(NSTimeInterval)delay
-                                  callback:(void (^)(void))callback;
+#pragma mark - FSTDelayedCallback
 
-/**
- * Queues the callback to run immediately (if it hasn't already been run or canceled).
- */
-- (void)skipDelay;
+@interface FSTDelayedCallback () {
+  DelayedOperation _impl;
+}
 
 @end
 
 @implementation FSTDelayedCallback
 
-- (instancetype)initWithQueue:(FSTDispatchQueue *)queue
-                      timerID:(FSTTimerID)timerID
-                   targetTime:(NSTimeInterval)targetTime
-                     callback:(void (^)(void))callback {
+- (instancetype)initWithImpl:(DelayedOperation &&)impl {
   if (self = [super init]) {
-    _queue = queue;
-    _timerID = timerID;
-    _targetTime = targetTime;
-    _callback = callback;
-    _done = NO;
+    _impl = std::move(impl);
   }
   return self;
 }
 
-+ (instancetype)createAndScheduleWithQueue:(FSTDispatchQueue *)queue
-                                   timerID:(FSTTimerID)timerID
-                                     delay:(NSTimeInterval)delay
-                                  callback:(void (^)(void))callback {
-  NSTimeInterval targetTime = [[NSDate date] timeIntervalSince1970] + delay;
-  FSTDelayedCallback *delayedCallback = [[FSTDelayedCallback alloc] initWithQueue:queue
-                                                                          timerID:timerID
-                                                                       targetTime:targetTime
-                                                                         callback:callback];
-  [delayedCallback startWithDelay:delay];
-  return delayedCallback;
-}
-
-/**
- * Starts the timer. This is called immediately after construction by createAndScheduleWithQueue.
- */
-- (void)startWithDelay:(NSTimeInterval)delay {
-  dispatch_time_t delayNs = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(delay * NSEC_PER_SEC));
-  dispatch_after(delayNs, self.queue.queue, ^{
-    [self.queue enterCheckedOperation:^{
-      [self delayDidElapse];
-    }];
-  });
-}
-
-- (void)skipDelay {
-  [self.queue dispatchAsyncAllowingSameQueue:^{
-    [self delayDidElapse];
-  }];
-}
-
 - (void)cancel {
-  [self.queue verifyIsCurrentQueue];
-  if (!self.isDone) {
-    // PORTING NOTE: There's no way to actually cancel the dispatched callback, but it'll be a no-op
-    // since we set done to YES.
-    [self markDone];
-  }
-}
-
-- (void)delayDidElapse {
-  [self.queue verifyIsCurrentQueue];
-  if (!self.isDone) {
-    [self markDone];
-    self.callback();
-  }
-}
-
-/**
- * Marks this delayed callback as done, and notifies the FSTDispatchQueue that it should be removed.
- */
-- (void)markDone {
-  self.done = YES;
-  [self.queue removeDelayedCallback:self];
+  _impl.Cancel();
 }
 
 @end
 
 #pragma mark - FSTDispatchQueue
 
-@interface FSTDispatchQueue ()
-/**
- * Callbacks scheduled to be queued in the future. Callbacks are automatically removed after they
- * are run or canceled.
- */
-@property(nonatomic, strong, readonly) NSMutableArray<FSTDelayedCallback *> *delayedCallbacks;
-
-- (instancetype)initWithQueue:(dispatch_queue_t)queue NS_DESIGNATED_INITIALIZER;
-
-@end
-
 @implementation FSTDispatchQueue {
-  /**
-   * Flag set while an FSTDispatchQueue operation is currently executing. Used for assertion
-   * sanity-checks.
-   */
-  std::atomic<bool> _operationInProgress;
+  std::unique_ptr<AsyncQueue> _impl;
+}
+
++ (TimerId)convertTimerId:(FSTTimerID)objcTimerID {
+  const TimerId converted = static_cast<TimerId>(objcTimerID);
+  switch (converted) {
+    case TimerId::All:
+    case TimerId::ListenStreamIdle:
+    case TimerId::ListenStreamConnectionBackoff:
+    case TimerId::WriteStreamIdle:
+    case TimerId::WriteStreamConnectionBackoff:
+    case TimerId::OnlineStateTimeout:
+      return converted;
+    default:
+      FSTAssert(false, @"Unknown value of enum FSTTimerID.");
+  }
 }
 
 + (instancetype)queueWith:(dispatch_queue_t)dispatchQueue {
@@ -172,141 +84,50 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (instancetype)initWithQueue:(dispatch_queue_t)queue {
   if (self = [super init]) {
-    _operationInProgress = false;
     _queue = queue;
-    _delayedCallbacks = [NSMutableArray array];
+    auto executor = absl::make_unique<ExecutorLibdispatch>(queue);
+    _impl = absl::make_unique<AsyncQueue>(std::move(executor));
   }
   return self;
 }
 
 - (void)verifyIsCurrentQueue {
-  FSTAssert([self onTargetQueue],
-            @"We are running on the wrong dispatch queue. Expected '%@' Actual: '%@'",
-            [self targetQueueLabel], [self currentQueueLabel]);
-  FSTAssert(_operationInProgress,
-            @"verifyIsCurrentQueue called outside enterCheckedOperation on queue '%@'",
-            [self currentQueueLabel]);
+  _impl->VerifyIsCurrentQueue();
 }
 
 - (void)enterCheckedOperation:(void (^)(void))block {
-  FSTAssert(!_operationInProgress,
-            @"enterCheckedOperation may not be called when an operation is in progress");
-  @try {
-    _operationInProgress = true;
-    [self verifyIsCurrentQueue];
-    block();
-  } @finally {
-    _operationInProgress = false;
-  }
+  _impl->ExecuteBlocking([block] { block(); });
 }
 
 - (void)dispatchAsync:(void (^)(void))block {
-  FSTAssert(![self onTargetQueue] || !_operationInProgress,
-            @"dispatchAsync called when we are already running on target dispatch queue '%@'",
-            [self targetQueueLabel]);
-
-  dispatch_async(self.queue, ^{
-    [self enterCheckedOperation:block];
-  });
+  _impl->Enqueue([block] { block(); });
 }
 
 - (void)dispatchAsyncAllowingSameQueue:(void (^)(void))block {
-  dispatch_async(self.queue, ^{
-    [self enterCheckedOperation:block];
-  });
+  _impl->EnqueueRelaxed([block] { block(); });
 }
 
 - (void)dispatchSync:(void (^)(void))block {
-  FSTAssert(![self onTargetQueue] || !_operationInProgress,
-            @"dispatchSync called when we are already running on target dispatch queue '%@'",
-            [self targetQueueLabel]);
-
-  dispatch_sync(self.queue, ^{
-    [self enterCheckedOperation:block];
-  });
+  _impl->EnqueueBlocking([block] { block(); });
 }
 
 - (FSTDelayedCallback *)dispatchAfterDelay:(NSTimeInterval)delay
                                    timerID:(FSTTimerID)timerID
                                      block:(void (^)(void))block {
-  // While not necessarily harmful, we currently don't expect to have multiple callbacks with the
-  // same timerID in the queue, so defensively reject them.
-  FSTAssert(![self containsDelayedCallbackWithTimerID:timerID],
-            @"Attempted to schedule multiple callbacks with id %ld", (unsigned long)timerID);
-  FSTDelayedCallback *delayedCallback = [FSTDelayedCallback createAndScheduleWithQueue:self
-                                                                               timerID:timerID
-                                                                                 delay:delay
-                                                                              callback:block];
-  [self.delayedCallbacks addObject:delayedCallback];
-  return delayedCallback;
+  const AsyncQueue::Milliseconds delayMs =
+      std::chrono::milliseconds(static_cast<long long>(delay * 1000));
+  const TimerId convertedTimerId = [FSTDispatchQueue convertTimerId:timerID];
+  DelayedOperation delayed_operation =
+      _impl->EnqueueAfterDelay(delayMs, convertedTimerId, [block] { block(); });
+  return [[FSTDelayedCallback alloc] initWithImpl:std::move(delayed_operation)];
 }
 
 - (BOOL)containsDelayedCallbackWithTimerID:(FSTTimerID)timerID {
-  NSUInteger matchIndex = [self.delayedCallbacks
-      indexOfObjectPassingTest:^BOOL(FSTDelayedCallback *obj, NSUInteger idx, BOOL *stop) {
-        return obj.timerID == timerID;
-      }];
-  return matchIndex != NSNotFound;
+  return _impl->IsScheduled([FSTDispatchQueue convertTimerId:timerID]);
 }
 
 - (void)runDelayedCallbacksUntil:(FSTTimerID)lastTimerID {
-  dispatch_semaphore_t doneSemaphore = dispatch_semaphore_create(0);
-
-  [self dispatchAsync:^{
-    FSTAssert(lastTimerID == FSTTimerIDAll || [self containsDelayedCallbackWithTimerID:lastTimerID],
-              @"Attempted to run callbacks until missing timer ID: %ld",
-              (unsigned long)lastTimerID);
-
-    [self sortDelayedCallbacks];
-    for (FSTDelayedCallback *callback in self.delayedCallbacks) {
-      [callback skipDelay];
-      if (lastTimerID != FSTTimerIDAll && callback.timerID == lastTimerID) {
-        break;
-      }
-    }
-
-    // Now that the callbacks are queued, we want to enqueue an additional item to release the
-    // 'done' semaphore.
-    [self dispatchAsyncAllowingSameQueue:^{
-      dispatch_semaphore_signal(doneSemaphore);
-    }];
-  }];
-
-  dispatch_semaphore_wait(doneSemaphore, DISPATCH_TIME_FOREVER);
-}
-
-// NOTE: For performance we could store the callbacks sorted (e.g. using std::priority_queue),
-// but this sort only happens in tests (if runDelayedCallbacksUntil: is called), and the size
-// is guaranteed to be small since we don't allow duplicate TimerIds (of which there are only 4).
-- (void)sortDelayedCallbacks {
-  // We want to run callbacks in the same order they'd run if they ran naturally.
-  [self.delayedCallbacks
-      sortUsingComparator:^NSComparisonResult(FSTDelayedCallback *a, FSTDelayedCallback *b) {
-        return a.targetTime < b.targetTime
-                   ? NSOrderedAscending
-                   : a.targetTime > b.targetTime ? NSOrderedDescending : NSOrderedSame;
-      }];
-}
-
-/** Called by FSTDelayedCallback when a callback is run or canceled. */
-- (void)removeDelayedCallback:(FSTDelayedCallback *)callback {
-  NSUInteger index = [self.delayedCallbacks indexOfObject:callback];
-  FSTAssert(index != NSNotFound, @"Delayed callback not found.");
-  [self.delayedCallbacks removeObjectAtIndex:index];
-}
-
-#pragma mark - Private Methods
-
-- (NSString *)currentQueueLabel {
-  return [NSString stringWithUTF8String:dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL)];
-}
-
-- (NSString *)targetQueueLabel {
-  return [NSString stringWithUTF8String:dispatch_queue_get_label(self.queue)];
-}
-
-- (BOOL)onTargetQueue {
-  return [[self currentQueueLabel] isEqualToString:[self targetQueueLabel]];
+  _impl->RunScheduledOperationsUntil([FSTDispatchQueue convertTimerId:lastTimerID]);
 }
 
 @end

Firestore/core/src/firebase/firestore/util/async_queue.cc

@@ -32,6 +32,8 @@ AsyncQueue::AsyncQueue(std::unique_ptr<Executor> executor)
   is_operation_in_progress_ = false;
 }
 
+// TODO(varconst): assert in destructor that the queue is empty.
+
 void AsyncQueue::VerifyIsCurrentExecutor() const {
   FIREBASE_ASSERT_MESSAGE(
       executor_->IsCurrentExecutor(),
@@ -97,8 +99,8 @@ void AsyncQueue::VerifySequentialOrder() const {
   // This is the inverse of `VerifyIsCurrentQueue`.
   FIREBASE_ASSERT_MESSAGE(
       !is_operation_in_progress_ || !executor_->IsCurrentExecutor(),
-      "Enforcing sequential order failed: currently executing operations "
-      "cannot enqueue more operations "
+      "Enqueue methods cannot be called when we are already running on "
+      "target executor"
       "(this queue's executor: '%s', current executor: '%s')",
       executor_->Name().c_str(), executor_->CurrentExecutorName().c_str());
 }

Firestore/core/src/firebase/firestore/util/executor_libdispatch.h

@@ -45,10 +45,10 @@ namespace internal {
 // Generic wrapper over `dispatch_async_f`, providing `dispatch_async`-like
 // interface: accepts an arbitrary invocable object in place of an Objective-C
 // block.
-void DispatchAsync(const dispatch_queue_t queue, std::function<void()>&& work);
+void DispatchAsync(dispatch_queue_t queue, std::function<void()>&& work);
 
 // Similar to `DispatchAsync` but wraps `dispatch_sync_f`.
-void DispatchSync(const dispatch_queue_t queue, std::function<void()> work);
+void DispatchSync(dispatch_queue_t queue, std::function<void()> work);
 
 class TimeSlot;
 
@@ -56,9 +56,7 @@ class TimeSlot;
 // a dedicated serial dispatch queue.
 class ExecutorLibdispatch : public Executor {
  public:
-  ExecutorLibdispatch();
   explicit ExecutorLibdispatch(dispatch_queue_t dispatch_queue);
-  ~ExecutorLibdispatch();
 
   bool IsCurrentExecutor() const override;
   std::string CurrentExecutorName() const override;
@@ -79,11 +77,6 @@ class ExecutorLibdispatch : public Executor {
   }
 
  private:
-  // GetLabel functions are guaranteed to never return a "null" string_view
-  // (i.e. data() != nullptr).
-  absl::string_view GetCurrentQueueLabel() const;
-  absl::string_view GetTargetQueueLabel() const;
-
   dispatch_queue_t dispatch_queue_;
   // Stores non-owned pointers to `TimeSlot`s.
   // Invariant: if a `TimeSlot` is in `schedule_`, it's a valid pointer.

Firestore/core/src/firebase/firestore/util/executor_libdispatch.mm

@@ -28,13 +28,16 @@ absl::string_view StringViewFromDispatchLabel(const char* const label) {
   return label ? absl::string_view{label} : absl::string_view{""};
 }
 
-void RunSynchronized(const ExecutorLibdispatch* const executor,
-                     std::function<void()>&& work) {
-  if (executor->IsCurrentExecutor()) {
-    work();
-  } else {
-    DispatchSync(executor->dispatch_queue(), std::move(work));
-  }
+// GetLabel functions are guaranteed to never return a "null" string_view
+// (i.e. data() != nullptr).
+absl::string_view GetQueueLabel(const dispatch_queue_t queue) {
+  return StringViewFromDispatchLabel(dispatch_queue_get_label(queue));
+}
+absl::string_view GetCurrentQueueLabel() {
+  // Note: dispatch_queue_get_label may return nullptr if the queue wasn't
+  // initialized with a label.
+  return StringViewFromDispatchLabel(
+      dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL));
 }
 
 }  // namespace
@@ -51,15 +54,32 @@ void DispatchAsync(const dispatch_queue_t queue, std::function<void()>&& work) {
   });
 }
 
-void DispatchSync(const dispatch_queue_t queue, Executor::Operation work) {
+void DispatchSync(const dispatch_queue_t queue, std::function<void()> work) {
+  FIREBASE_ASSERT_MESSAGE(
+      GetCurrentQueueLabel() != GetQueueLabel(queue),
+      "Calling DispatchSync on the current queue will lead to a deadlock.");
+
   // Unlike dispatch_async_f, dispatch_sync_f blocks until the work passed to it
   // is done, so passing a reference to a local variable is okay.
   dispatch_sync_f(queue, &work, [](void* const raw_work) {
-    const auto unwrap = static_cast<const Executor::Operation*>(raw_work);
+    const auto unwrap = static_cast<std::function<void()>*>(raw_work);
     (*unwrap)();
   });
 }
 
+namespace {
+
+template <typename Work>
+void RunSynchronized(const ExecutorLibdispatch* const executor, Work&& work) {
+  if (executor->IsCurrentExecutor()) {
+    work();
+  } else {
+    DispatchSync(executor->dispatch_queue(), std::forward<Work>(work));
+  }
+}
+
+}  // namespace
+
 // Represents a "busy" time slot on the schedule.
 //
 // Since libdispatch doesn't provide a way to cancel a scheduled operation, once
@@ -170,32 +190,15 @@ void TimeSlot::RemoveFromSchedule() {
 ExecutorLibdispatch::ExecutorLibdispatch(const dispatch_queue_t dispatch_queue)
     : dispatch_queue_{dispatch_queue} {
 }
-ExecutorLibdispatch::ExecutorLibdispatch()
-    : ExecutorLibdispatch{dispatch_queue_create("com.google.firebase.firestore",
-                                                DISPATCH_QUEUE_SERIAL)} {
-}
-
-ExecutorLibdispatch::~ExecutorLibdispatch() {
-  // Turn any operations that might still be in the queue into no-ops, lest
-  // they try to access `ExecutorLibdispatch` after it gets destroyed. Because
-  // the queue is serial, by the time libdispatch gets to the newly-enqueued
-  // work, the pending operations that might have been in progress would have
-  // already finished.
-  RunSynchronized(this, [this] {
-    for (auto slot : schedule_) {
-      slot->MarkDone();
-    }
-  });
-}
 
 bool ExecutorLibdispatch::IsCurrentExecutor() const {
-  return GetCurrentQueueLabel().data() == GetTargetQueueLabel().data();
+  return GetCurrentQueueLabel() == GetQueueLabel(dispatch_queue());
 }
 std::string ExecutorLibdispatch::CurrentExecutorName() const {
   return GetCurrentQueueLabel().data();
 }
 std::string ExecutorLibdispatch::Name() const {
-  return GetTargetQueueLabel().data();
+  return GetQueueLabel(dispatch_queue()).data();
 }
 
 void ExecutorLibdispatch::Execute(Operation&& operation) {
@@ -243,29 +246,14 @@ void ExecutorLibdispatch::RemoveFromSchedule(const TimeSlot* const to_remove) {
   });
 }
 
-// GetLabel functions are guaranteed to never return a "null" string_view
-// (i.e. data() != nullptr).
-absl::string_view ExecutorLibdispatch::GetCurrentQueueLabel() const {
-  // Note: dispatch_queue_get_label may return nullptr if the queue wasn't
-  // initialized with a label.
-  return StringViewFromDispatchLabel(
-      dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL));
-}
-
-absl::string_view ExecutorLibdispatch::GetTargetQueueLabel() const {
-  return StringViewFromDispatchLabel(
-      dispatch_queue_get_label(dispatch_queue()));
-}
-
 // Test-only methods
 
 bool ExecutorLibdispatch::IsScheduled(const Tag tag) const {
   bool result = false;
   RunSynchronized(this, [this, tag, &result] {
-    result = std::find_if(schedule_.begin(), schedule_.end(),
-                          [&tag](const TimeSlot* const operation) {
-                            return *operation == tag;
-                          }) != schedule_.end();
+    result = std::any_of(
+        schedule_.begin(), schedule_.end(),
+        [&tag](const TimeSlot* const operation) { return *operation == tag; });
   });
   return result;
 }

Firestore/core/src/firebase/firestore/util/executor_std.h

@@ -220,7 +220,6 @@ class ExecutorStd : public Executor {
   bool IsScheduled(Tag tag) const override;
   absl::optional<TaggedOperation> PopFromSchedule() override;
 
- private:
   using TimePoint = async::Schedule<Operation>::TimePoint;
   // To allow canceling operations, each scheduled operation is assigned
   // a monotonically increasing identifier.

Firestore/core/test/firebase/firestore/util/async_queue_libdispatch_test.mm

@@ -77,7 +77,7 @@ TEST_F(AsyncQueueTestLibdispatchOnly,
 }
 
 TEST_F(AsyncQueueTestLibdispatchOnly,
-       VerifyIsCurrentQueueRequiresBeingCalledAsync) {
+       VerifyIsCurrentQueueRequiresBeingCalledOnTheQueue) {
   ASSERT_NE(underlying_queue, dispatch_get_main_queue());
   EXPECT_ANY_THROW(queue.VerifyIsCurrentQueue());
 }

Firestore/core/test/firebase/firestore/util/executor_libdispatch_test.mm

@@ -29,7 +29,8 @@ namespace util {
 namespace {
 
 std::unique_ptr<internal::Executor> ExecutorFactory() {
-  return absl::make_unique<internal::ExecutorLibdispatch>();
+  return absl::make_unique<internal::ExecutorLibdispatch>(
+      dispatch_queue_create("ExecutorLibdispatchTests", DISPATCH_QUEUE_SERIAL));
 }
 
 }  // namespace
@@ -38,6 +39,36 @@ INSTANTIATE_TEST_CASE_P(ExecutorTestLibdispatch,
                         ExecutorTest,
                         ::testing::Values(ExecutorFactory));
 
+namespace internal {
+class ExecutorLibdispatchOnlyTests : public TestWithTimeoutMixin,
+                                     public ::testing::Test {
+ public:
+  ExecutorLibdispatchOnlyTests() : executor{ExecutorFactory()} {
+  }
+
+  std::unique_ptr<Executor> executor;
+};
+
+TEST_F(ExecutorLibdispatchOnlyTests, NameReturnsLabelOfTheQueue) {
+  EXPECT_EQ(executor->Name(), "ExecutorLibdispatchTests");
+  executor->Execute([&] {
+    EXPECT_EQ(executor->CurrentExecutorName(), "ExecutorLibdispatchTests");
+    signal_finished();
+  });
+  EXPECT_TRUE(WaitForTestToFinish());
+}
+
+TEST_F(ExecutorLibdispatchOnlyTests,
+       ExecuteBlockingOnTheCurrentQueueIsNotAllowed) {
+  EXPECT_NO_THROW(executor->ExecuteBlocking([] {}));
+  executor->Execute([&] {
+    EXPECT_ANY_THROW(executor->ExecuteBlocking([] {}));
+    signal_finished();
+  });
+  EXPECT_TRUE(WaitForTestToFinish());
+}
+
+}  // namespace internal
 }  // namespace util
 }  // namespace firestore
 }  // namespace firebase