From edd6f60a7385f9280c371df273703f6e104e325f Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Thu, 10 May 2018 20:16:50 -0400 Subject: Firestore: use C++ Executor for user queue. (#1238) FSTDispatchQueue enforces serial execution, which is inappropriate for user queue, because a user may configure usage of a concurrent queue in settings, breaking FSTDispatchQueue invariants. Instead, use C++ ExecutorLibdispatch directly. --- Firestore/Source/Core/FSTFirestoreClient.h | 19 +++++++----- Firestore/Source/Core/FSTFirestoreClient.mm | 45 ++++++++++++++--------------- 2 files changed, 32 insertions(+), 32 deletions(-) (limited to 'Firestore/Source/Core') diff --git a/Firestore/Source/Core/FSTFirestoreClient.h b/Firestore/Source/Core/FSTFirestoreClient.h index 7285e65..94c2284 100644 --- a/Firestore/Source/Core/FSTFirestoreClient.h +++ b/Firestore/Source/Core/FSTFirestoreClient.h @@ -15,6 +15,7 @@ */ #import +#include #import "Firestore/Source/Core/FSTTypes.h" #import "Firestore/Source/Core/FSTViewSnapshot.h" @@ -23,6 +24,7 @@ #include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h" #include "Firestore/core/src/firebase/firestore/core/database_info.h" #include "Firestore/core/src/firebase/firestore/model/database_id.h" +#include "Firestore/core/src/firebase/firestore/util/executor.h" @class FIRDocumentReference; @class FIRDocumentSnapshot; @@ -50,14 +52,15 @@ NS_ASSUME_NONNULL_BEGIN /** * Creates and returns a FSTFirestoreClient with the given parameters. * - * All callbacks and events will be triggered on the provided userDispatchQueue. + * All callbacks and events will be triggered on the provided userExecutor. */ -+ (instancetype)clientWithDatabaseInfo:(const firebase::firestore::core::DatabaseInfo &)databaseInfo - usePersistence:(BOOL)usePersistence - credentialsProvider:(firebase::firestore::auth::CredentialsProvider *) - credentialsProvider // no passing ownership - userDispatchQueue:(FSTDispatchQueue *)userDispatchQueue - workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue; ++ (instancetype) +clientWithDatabaseInfo:(const firebase::firestore::core::DatabaseInfo &)databaseInfo + usePersistence:(BOOL)usePersistence + credentialsProvider:(firebase::firestore::auth::CredentialsProvider *) + credentialsProvider // no passing ownership + userExecutor:(std::unique_ptr)userExecutor + workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue; - (instancetype)init __attribute__((unavailable("Use static constructor method."))); @@ -111,7 +114,7 @@ NS_ASSUME_NONNULL_BEGIN * Dispatch queue for user callbacks / events. This will often be the "Main Dispatch Queue" of the * app but the developer can configure it to a different queue if they so choose. */ -@property(nonatomic, strong, readonly) FSTDispatchQueue *userDispatchQueue; +- (firebase::firestore::util::internal::Executor *)userExecutor; @end diff --git a/Firestore/Source/Core/FSTFirestoreClient.mm b/Firestore/Source/Core/FSTFirestoreClient.mm index 658cf57..93d1f7c 100644 --- a/Firestore/Source/Core/FSTFirestoreClient.mm +++ b/Firestore/Source/Core/FSTFirestoreClient.mm @@ -18,6 +18,7 @@ #include // NOLINT(build/c++11) #include +#include #import "FIRFirestoreErrors.h" #import "Firestore/Source/API/FIRDocumentReference+Internal.h" @@ -58,6 +59,8 @@ using firebase::firestore::core::DatabaseInfo; using firebase::firestore::model::DatabaseId; using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::util::internal::Executor; + NS_ASSUME_NONNULL_BEGIN @interface FSTFirestoreClient () { @@ -68,7 +71,7 @@ NS_ASSUME_NONNULL_BEGIN usePersistence:(BOOL)usePersistence credentialsProvider: (CredentialsProvider *)credentialsProvider // no passing ownership - userDispatchQueue:(FSTDispatchQueue *)userDispatchQueue + userExecutor:(std::unique_ptr)userExecutor workerDispatchQueue:(FSTDispatchQueue *)queue NS_DESIGNATED_INITIALIZER; @property(nonatomic, assign, readonly) const DatabaseInfo *databaseInfo; @@ -91,18 +94,24 @@ NS_ASSUME_NONNULL_BEGIN @end -@implementation FSTFirestoreClient +@implementation FSTFirestoreClient { + std::unique_ptr _userExecutor; +} + +- (Executor *)userExecutor { + return _userExecutor.get(); +} + (instancetype)clientWithDatabaseInfo:(const DatabaseInfo &)databaseInfo usePersistence:(BOOL)usePersistence credentialsProvider: (CredentialsProvider *)credentialsProvider // no passing ownership - userDispatchQueue:(FSTDispatchQueue *)userDispatchQueue + userExecutor:(std::unique_ptr)userExecutor workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue { return [[FSTFirestoreClient alloc] initWithDatabaseInfo:databaseInfo usePersistence:usePersistence credentialsProvider:credentialsProvider - userDispatchQueue:userDispatchQueue + userExecutor:std::move(userExecutor) workerDispatchQueue:workerDispatchQueue]; } @@ -110,12 +119,12 @@ NS_ASSUME_NONNULL_BEGIN usePersistence:(BOOL)usePersistence credentialsProvider: (CredentialsProvider *)credentialsProvider // no passing ownership - userDispatchQueue:(FSTDispatchQueue *)userDispatchQueue + userExecutor:(std::unique_ptr)userExecutor workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue { if (self = [super init]) { _databaseInfo = databaseInfo; _credentialsProvider = credentialsProvider; - _userDispatchQueue = userDispatchQueue; + _userExecutor = std::move(userExecutor); _workerDispatchQueue = workerDispatchQueue; auto userPromise = std::make_shared>(); @@ -230,9 +239,7 @@ NS_ASSUME_NONNULL_BEGIN [self.workerDispatchQueue dispatchAsync:^{ [self.remoteStore disableNetwork]; if (completion) { - [self.userDispatchQueue dispatchAsync:^{ - completion(nil); - }]; + _userExecutor->Execute([=] { completion(nil); }); } }]; } @@ -241,9 +248,7 @@ NS_ASSUME_NONNULL_BEGIN [self.workerDispatchQueue dispatchAsync:^{ [self.remoteStore enableNetwork]; if (completion) { - [self.userDispatchQueue dispatchAsync:^{ - completion(nil); - }]; + _userExecutor->Execute([=] { completion(nil); }); } }]; } @@ -255,9 +260,7 @@ NS_ASSUME_NONNULL_BEGIN [self.remoteStore shutdown]; [self.persistence shutdown]; if (completion) { - [self.userDispatchQueue dispatchAsync:^{ - completion(nil); - }]; + _userExecutor->Execute([=] { completion(nil); }); } }]; } @@ -338,18 +341,14 @@ NS_ASSUME_NONNULL_BEGIN [self.workerDispatchQueue dispatchAsync:^{ if (mutations.count == 0) { if (completion) { - [self.userDispatchQueue dispatchAsync:^{ - completion(nil); - }]; + _userExecutor->Execute([=] { completion(nil); }); } } else { [self.syncEngine writeMutations:mutations completion:^(NSError *error) { // Dispatch the result back onto the user dispatch queue. if (completion) { - [self.userDispatchQueue dispatchAsync:^{ - completion(error); - }]; + _userExecutor->Execute([=] { completion(error); }); } }]; } @@ -366,9 +365,7 @@ NS_ASSUME_NONNULL_BEGIN completion:^(id _Nullable result, NSError *_Nullable error) { // Dispatch the result back onto the user dispatch queue. if (completion) { - [self.userDispatchQueue dispatchAsync:^{ - completion(result, error); - }]; + _userExecutor->Execute([=] { completion(result, error); }); } }]; }]; -- cgit v1.2.3