diff options
8 files changed, 150 insertions, 306 deletions
diff --git a/Firestore/Example/Tests/Util/FSTDispatchQueueTests.mm b/Firestore/Example/Tests/Util/FSTDispatchQueueTests.mm index 811fa34..1f49aa4 100644 --- a/Firestore/Example/Tests/Util/FSTDispatchQueueTests.mm +++ b/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]; diff --git a/Firestore/Source/Util/FSTDispatchQueue.mm b/Firestore/Source/Util/FSTDispatchQueue.mm index 0974359..01b2732 100644 --- a/Firestore/Source/Util/FSTDispatchQueue.mm +++ b/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 diff --git a/Firestore/core/src/firebase/firestore/util/async_queue.cc b/Firestore/core/src/firebase/firestore/util/async_queue.cc index 71f5cc5..81aac7c 100644 --- a/Firestore/core/src/firebase/firestore/util/async_queue.cc +++ b/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()); } diff --git a/Firestore/core/src/firebase/firestore/util/executor_libdispatch.h b/Firestore/core/src/firebase/firestore/util/executor_libdispatch.h index 85c34f8..f913fe5 100644 --- a/Firestore/core/src/firebase/firestore/util/executor_libdispatch.h +++ b/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. diff --git a/Firestore/core/src/firebase/firestore/util/executor_libdispatch.mm b/Firestore/core/src/firebase/firestore/util/executor_libdispatch.mm index 5491fec..597d450 100644 --- a/Firestore/core/src/firebase/firestore/util/executor_libdispatch.mm +++ b/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; } diff --git a/Firestore/core/src/firebase/firestore/util/executor_std.h b/Firestore/core/src/firebase/firestore/util/executor_std.h index 58ef96f..8f2be02 100644 --- a/Firestore/core/src/firebase/firestore/util/executor_std.h +++ b/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. diff --git a/Firestore/core/test/firebase/firestore/util/async_queue_libdispatch_test.mm b/Firestore/core/test/firebase/firestore/util/async_queue_libdispatch_test.mm index 5452266..f1ff394 100644 --- a/Firestore/core/test/firebase/firestore/util/async_queue_libdispatch_test.mm +++ b/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()); } diff --git a/Firestore/core/test/firebase/firestore/util/executor_libdispatch_test.mm b/Firestore/core/test/firebase/firestore/util/executor_libdispatch_test.mm index 0167c83..330c8fc 100644 --- a/Firestore/core/test/firebase/firestore/util/executor_libdispatch_test.mm +++ b/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 |