From 51324d98781f2bb0e4e58f088899427ab2f212f2 Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Wed, 3 Mar 2021 14:17:31 +0900 Subject: [PATCH 01/22] implement thread pool executor service --- .../common/UniversalFirebaseModule.java | 52 ++++++++++++++++--- .../common/ReactNativeFirebaseModule.java | 52 ++++++++++++++++--- 2 files changed, 90 insertions(+), 14 deletions(-) diff --git a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java index 0e1b6ab01d..2c3f9e9c9a 100644 --- a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java +++ b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java @@ -23,6 +23,10 @@ import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.SynchronousQueue; import javax.annotation.OverridingMethodsMustInvokeSuper; @@ -45,12 +49,38 @@ public Context getApplicationContext() { return getContext().getApplicationContext(); } + // TODO: remove this protected ExecutorService getExecutor() { - ExecutorService existingSingleThreadExecutor = executors.get(getName()); - if (existingSingleThreadExecutor != null) return existingSingleThreadExecutor; - ExecutorService newSingleThreadExecutor = Executors.newSingleThreadExecutor(); - executors.put(getName(), newSingleThreadExecutor); - return newSingleThreadExecutor; + return getExecutor(false); + } + + protected ExecutorService getTransactionalExecutor() { + return getExecutor(true); + } + + private ExecutorService getExecutor(boolean isTransactional) { + String executorName = getExecutorName(isTransactional); + ExecutorService existingExecutor = executors.get(executorName); + if (existingExecutor != null) return existingExecutor; + ExecutorService newExecutor = getNewExecutor(isTransactional); + executors.put(executorName, newExecutor); + return newExecutor; + } + + private ExecutorService getNewExecutor(boolean isTransactional) { + if (isTransactional == true) { + return new ThreadPoolExecutor(0, 1, 60L, TimeUnit.SECONDS, new LinkedBlockingQueue()); + } else { + return new ThreadPoolExecutor(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue()); + } + } + + private String getExecutorName(Boolean isTransactional) { + String moduleName = getName(); + if (isTransactional == true) { + return moduleName + "TransactionalExecutor"; + } + return moduleName + "Executor"; } public String getName() { @@ -59,10 +89,18 @@ public String getName() { @OverridingMethodsMustInvokeSuper public void onTearDown() { - ExecutorService existingSingleThreadExecutor = executors.get(getName()); + String singleThreadExecutorName = getExecutorName(false); + ExecutorService existingSingleThreadExecutor = executors.get(singleThreadExecutorName); if (existingSingleThreadExecutor != null) { existingSingleThreadExecutor.shutdownNow(); - executors.remove(getName()); + executors.remove(singleThreadExecutorName); + } + + String threadPoolExecutorName = getExecutorName(false); + ExecutorService existingThreadPoolExecutor = executors.get(threadPoolExecutorName); + if (existingThreadPoolExecutor != null) { + existingThreadPoolExecutor.shutdownNow(); + executors.remove(threadPoolExecutorName); } } diff --git a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java index 8f343de599..5bb7489cb1 100644 --- a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java +++ b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java @@ -27,6 +27,10 @@ import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.SynchronousQueue; public class ReactNativeFirebaseModule extends ReactContextBaseJavaModule implements ContextProvider { private static Map executors = new HashMap<>(); @@ -73,20 +77,54 @@ public ReactContext getContext() { return getReactApplicationContext(); } + // TODO: remove this public ExecutorService getExecutor() { - ExecutorService existingSingleThreadExecutor = executors.get(getName()); - if (existingSingleThreadExecutor != null) return existingSingleThreadExecutor; - ExecutorService newSingleThreadExecutor = Executors.newSingleThreadExecutor(); - executors.put(getName(), newSingleThreadExecutor); - return newSingleThreadExecutor; + return getExecutor(false); + } + + public ExecutorService getTransactionalExecutor() { + return getExecutor(true); + } + + private ExecutorService getExecutor(boolean isTransactional) { + String executorName = getExecutorName(isTransactional); + ExecutorService existingExecutor = executors.get(executorName); + if (existingExecutor != null) return existingExecutor; + ExecutorService newExecutor = getNewExecutor(isTransactional); + executors.put(executorName, newExecutor); + return newExecutor; + } + + private ExecutorService getNewExecutor(boolean isTransactional) { + if (isTransactional == true) { + return new ThreadPoolExecutor(0, 1, 60L, TimeUnit.SECONDS, new LinkedBlockingQueue()); + } else { + return new ThreadPoolExecutor(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue()); + } + } + + private String getExecutorName(boolean isTransactional) { + String moduleName = getName(); + if (isTransactional == true) { + return moduleName + "TransactionalExecutor"; + } + return moduleName + "Executor"; } @Override public void onCatalystInstanceDestroy() { - ExecutorService existingSingleThreadExecutor = executors.get(getName()); + String singleThreadExecutorName = getExecutorName(false); + ExecutorService existingSingleThreadExecutor = executors.get(singleThreadExecutorName); if (existingSingleThreadExecutor != null) { existingSingleThreadExecutor.shutdownNow(); - executors.remove(getName()); + executors.remove(singleThreadExecutorName); + } + + String threadPoolExecutorName = getExecutorName(false); + ExecutorService existingThreadPoolExecutor = executors.get(threadPoolExecutorName); + if (existingThreadPoolExecutor != null) { + existingThreadPoolExecutor.shutdownNow(); + executors.remove(threadPoolExecutorName); } } From 68090265ff95b9217b7cf1460c8e1c9294afa058 Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Wed, 3 Mar 2021 14:33:38 +0900 Subject: [PATCH 02/22] add getTransactionalExecutor method --- .../io/invertase/firebase/common/UniversalFirebaseModule.java | 1 - .../io/invertase/firebase/common/ReactNativeFirebaseModule.java | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java index 2c3f9e9c9a..04ceba4bc9 100644 --- a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java +++ b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java @@ -49,7 +49,6 @@ public Context getApplicationContext() { return getContext().getApplicationContext(); } - // TODO: remove this protected ExecutorService getExecutor() { return getExecutor(false); } diff --git a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java index 5bb7489cb1..b787cc8070 100644 --- a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java +++ b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java @@ -77,7 +77,6 @@ public ReactContext getContext() { return getReactApplicationContext(); } - // TODO: remove this public ExecutorService getExecutor() { return getExecutor(false); } From f1f1e37a8f8149960a533062d5ed8b54a6c106b2 Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Wed, 3 Mar 2021 14:43:52 +0900 Subject: [PATCH 03/22] use transactional executor during write action --- ...actNativeFirebaseDatabaseReferenceModule.java | 16 ++++++++-------- ...actNativeFirebaseFirestoreDocumentModule.java | 12 ++++++------ ...NativeFirebaseFirestoreTransactionModule.java | 2 +- .../ReactNativeFirebaseStorageModule.java | 12 ++++++------ 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/database/android/src/reactnative/java/io/invertase/firebase/database/ReactNativeFirebaseDatabaseReferenceModule.java b/packages/database/android/src/reactnative/java/io/invertase/firebase/database/ReactNativeFirebaseDatabaseReferenceModule.java index fb5556ffeb..6e5e440041 100644 --- a/packages/database/android/src/reactnative/java/io/invertase/firebase/database/ReactNativeFirebaseDatabaseReferenceModule.java +++ b/packages/database/android/src/reactnative/java/io/invertase/firebase/database/ReactNativeFirebaseDatabaseReferenceModule.java @@ -41,9 +41,9 @@ public class ReactNativeFirebaseDatabaseReferenceModule extends ReactNativeFireb @ReactMethod public void set(String app, String dbURL, String path, ReadableMap props, Promise promise) { Tasks - .call(getExecutor(), () -> toHashMap(props).get("value")) + .call(getTransactionalExecutor(), () -> toHashMap(props).get("value")) .onSuccessTask(aValue -> module.set(app, dbURL, path, aValue)) - .addOnCompleteListener(getExecutor(), task -> { + .addOnCompleteListener(getTransactionalExecutor(), task -> { if (task.isSuccessful()) { promise.resolve(task.getResult()); } else { @@ -56,9 +56,9 @@ public void set(String app, String dbURL, String path, ReadableMap props, Promis @ReactMethod public void update(String app, String dbURL, String path, ReadableMap props, Promise promise) { Tasks - .call(getExecutor(), () -> toHashMap(props).get("values")) + .call(getTransactionalExecutor(), () -> toHashMap(props).get("values")) .onSuccessTask(aMap -> module.update(app, dbURL, path, (Map) aMap)) - .addOnCompleteListener(getExecutor(), task -> { + .addOnCompleteListener(getTransactionalExecutor(), task -> { if (task.isSuccessful()) { promise.resolve(task.getResult()); } else { @@ -70,9 +70,9 @@ public void update(String app, String dbURL, String path, ReadableMap props, Pro @ReactMethod public void setWithPriority(String app, String dbURL, String path, ReadableMap props, Promise promise) { Tasks - .call(getExecutor(), () -> toHashMap(props)) + .call(getTransactionalExecutor(), () -> toHashMap(props)) .onSuccessTask(aMap -> module.setWithPriority(app, dbURL, path, aMap.get("value"), aMap.get("priority"))) - .addOnCompleteListener(getExecutor(), task -> { + .addOnCompleteListener(getTransactionalExecutor(), task -> { if (task.isSuccessful()) { promise.resolve(task.getResult()); } else { @@ -85,7 +85,7 @@ public void setWithPriority(String app, String dbURL, String path, ReadableMap p public void remove(String app, String dbURL, String path, Promise promise) { // continuation tasks not needed for this as no data module.remove(app, dbURL, path) - .addOnCompleteListener(getExecutor(), task -> { + .addOnCompleteListener(getTransactionalExecutor(), task -> { if (task.isSuccessful()) { promise.resolve(task.getResult()); } else { @@ -98,7 +98,7 @@ public void remove(String app, String dbURL, String path, Promise promise) { public void setPriority(String app, String dbURL, String path, ReadableMap props, Promise promise) { // continuation tasks not needed for this as minimal data module.setPriority(app, dbURL, path, toHashMap(props).get("priority")) - .addOnCompleteListener(getExecutor(), task -> { + .addOnCompleteListener(getTransactionalExecutor(), task -> { if (task.isSuccessful()) { promise.resolve(task.getResult()); } else { diff --git a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreDocumentModule.java b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreDocumentModule.java index a213a787e4..58dff5ee6f 100644 --- a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreDocumentModule.java +++ b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreDocumentModule.java @@ -145,7 +145,7 @@ public void documentGet(String appName, String path, ReadableMap getOptions, Pro public void documentDelete(String appName, String path, Promise promise) { FirebaseFirestore firebaseFirestore = getFirestoreForApp(appName); DocumentReference documentReference = getDocumentForFirestore(firebaseFirestore, path); - Tasks.call(getExecutor(), documentReference::delete).addOnCompleteListener(task -> { + Tasks.call(getTransactionalExecutor(), documentReference::delete).addOnCompleteListener(task -> { if (task.isSuccessful()) { promise.resolve(null); } else { @@ -160,7 +160,7 @@ public void documentSet(String appName, String path, ReadableMap data, ReadableM DocumentReference documentReference = getDocumentForFirestore(firebaseFirestore, path); - Tasks.call(getExecutor(), () -> parseReadableMap(firebaseFirestore, data)).continueWithTask(getExecutor(), task -> { + Tasks.call(getTransactionalExecutor(), () -> parseReadableMap(firebaseFirestore, data)).continueWithTask(getTransactionalExecutor(), task -> { Task setTask; Map settableData = Objects.requireNonNull(task.getResult()); @@ -193,8 +193,8 @@ public void documentUpdate(String appName, String path, ReadableMap data, Promis FirebaseFirestore firebaseFirestore = getFirestoreForApp(appName); DocumentReference documentReference = getDocumentForFirestore(firebaseFirestore, path); - Tasks.call(getExecutor(), () -> parseReadableMap(firebaseFirestore, data)) - .continueWithTask(getExecutor(), task -> documentReference.update(Objects.requireNonNull(task.getResult()))) + Tasks.call(getTransactionalExecutor(), () -> parseReadableMap(firebaseFirestore, data)) + .continueWithTask(getTransactionalExecutor(), task -> documentReference.update(Objects.requireNonNull(task.getResult()))) .addOnCompleteListener(task -> { if (task.isSuccessful()) { promise.resolve(null); @@ -208,8 +208,8 @@ public void documentUpdate(String appName, String path, ReadableMap data, Promis public void documentBatch(String appName, ReadableArray writes, Promise promise) { FirebaseFirestore firebaseFirestore = getFirestoreForApp(appName); - Tasks.call(getExecutor(), () -> parseDocumentBatches(firebaseFirestore, writes)) - .continueWithTask(getExecutor(), task -> { + Tasks.call(getTransactionalExecutor(), () -> parseDocumentBatches(firebaseFirestore, writes)) + .continueWithTask(getTransactionalExecutor(), task -> { WriteBatch batch = firebaseFirestore.batch(); List writesArray = task.getResult(); diff --git a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreTransactionModule.java b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreTransactionModule.java index b454eaf101..8b5f9a37c6 100644 --- a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreTransactionModule.java +++ b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreTransactionModule.java @@ -71,7 +71,7 @@ public void transactionGetDocument(String appName, int transactionId, String pat DocumentReference documentReference = getDocumentForFirestore(firebaseFirestore, path); Tasks - .call(getExecutor(), () -> snapshotToWritableMap(transactionHandler.getDocument(documentReference))) + .call(getTransactionalExecutor(), () -> snapshotToWritableMap(transactionHandler.getDocument(documentReference))) .addOnCompleteListener(task -> { if (task.isSuccessful()) { promise.resolve(task.getResult()); diff --git a/packages/storage/android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageModule.java b/packages/storage/android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageModule.java index bad4dce432..4f887f6b41 100644 --- a/packages/storage/android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageModule.java +++ b/packages/storage/android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageModule.java @@ -221,8 +221,8 @@ public void writeToFile( reference, appName ); - storageTask.begin(getExecutor(), localFilePath); - storageTask.addOnCompleteListener(getExecutor(), promise); + storageTask.begin(getTransactionalExecutor(), localFilePath); + storageTask.addOnCompleteListener(getTransactionalExecutor(), promise); } /** @@ -244,8 +244,8 @@ public void putString( reference, appName ); - storageTask.begin(getExecutor(), string, format, metadataMap); - storageTask.addOnCompleteListener(getExecutor(),promise); + storageTask.begin(getTransactionalExecutor(), string, format, metadataMap); + storageTask.addOnCompleteListener(getTransactionalExecutor(),promise); } /** @@ -266,8 +266,8 @@ public void putFile( reference, appName ); - storageTask.begin(getExecutor(),localFilePath, metadata); - storageTask.addOnCompleteListener(getExecutor(), promise); + storageTask.begin(getTransactionalExecutor(),localFilePath, metadata); + storageTask.addOnCompleteListener(getTransactionalExecutor(), promise); } @ReactMethod From ea9106e21b8b9bf86ee1d7a785520efde3c29c99 Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Wed, 3 Mar 2021 16:19:53 +0900 Subject: [PATCH 04/22] tune thread pool queuing --- .../common/UniversalFirebaseModule.java | 17 ++++++++++++++--- .../common/ReactNativeFirebaseModule.java | 17 ++++++++++++++--- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java index 04ceba4bc9..ff2c0fb58c 100644 --- a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java +++ b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java @@ -23,14 +23,16 @@ import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.RejectedExecutionHandler; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; -import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.SynchronousQueue; import javax.annotation.OverridingMethodsMustInvokeSuper; public class UniversalFirebaseModule { + private static final int MAXIMUM_POOL_SIZE = 20; + private static final int KEEP_ALIVE_SECONDS = 3; private static Map executors = new HashMap<>(); private final Context context; @@ -68,12 +70,21 @@ private ExecutorService getExecutor(boolean isTransactional) { private ExecutorService getNewExecutor(boolean isTransactional) { if (isTransactional == true) { - return new ThreadPoolExecutor(0, 1, 60L, TimeUnit.SECONDS, new LinkedBlockingQueue()); + return Executors.newSingleThreadExecutor(); } else { - return new ThreadPoolExecutor(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue()); + ThreadPoolExecutor threadPoolExecutor = new ThreadPoolExecutor(0, MAXIMUM_POOL_SIZE, KEEP_ALIVE_SECONDS, TimeUnit.SECONDS, new SynchronousQueue()); + threadPoolExecutor.setRejectedExecutionHandler(executeInFallback); + return threadPoolExecutor; } } + private RejectedExecutionHandler executeInFallback = new RejectedExecutionHandler() { + public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { + ExecutorService fallbackExecutor = getTransactionalExecutor(); + fallbackExecutor.execute(r); + }; + }; + private String getExecutorName(Boolean isTransactional) { String moduleName = getName(); if (isTransactional == true) { diff --git a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java index b787cc8070..f51c6b9f95 100644 --- a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java +++ b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java @@ -27,12 +27,14 @@ import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.RejectedExecutionHandler; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; -import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.SynchronousQueue; public class ReactNativeFirebaseModule extends ReactContextBaseJavaModule implements ContextProvider { + private static final int MAXIMUM_POOL_SIZE = 20; + private static final int KEEP_ALIVE_SECONDS = 3; private static Map executors = new HashMap<>(); private String moduleName; @@ -96,12 +98,21 @@ private ExecutorService getExecutor(boolean isTransactional) { private ExecutorService getNewExecutor(boolean isTransactional) { if (isTransactional == true) { - return new ThreadPoolExecutor(0, 1, 60L, TimeUnit.SECONDS, new LinkedBlockingQueue()); + return Executors.newSingleThreadExecutor(); } else { - return new ThreadPoolExecutor(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue()); + ThreadPoolExecutor threadPoolExecutor = new ThreadPoolExecutor(0, MAXIMUM_POOL_SIZE, KEEP_ALIVE_SECONDS, TimeUnit.SECONDS, new SynchronousQueue()); + threadPoolExecutor.setRejectedExecutionHandler(executeInFallback); + return threadPoolExecutor; } } + private RejectedExecutionHandler executeInFallback = new RejectedExecutionHandler() { + public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { + ExecutorService fallbackExecutor = getTransactionalExecutor(); + fallbackExecutor.execute(r); + }; + }; + private String getExecutorName(boolean isTransactional) { String moduleName = getName(); if (isTransactional == true) { From ad5a82b7d154991a5d3a0fe0d005b4e4d6e7397a Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Wed, 3 Mar 2021 16:33:25 +0900 Subject: [PATCH 05/22] fix tear down method --- .../common/UniversalFirebaseModule.java | 20 +++++++++---------- .../common/ReactNativeFirebaseModule.java | 20 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java index ff2c0fb58c..0777beef0c 100644 --- a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java +++ b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java @@ -99,18 +99,18 @@ public String getName() { @OverridingMethodsMustInvokeSuper public void onTearDown() { - String singleThreadExecutorName = getExecutorName(false); - ExecutorService existingSingleThreadExecutor = executors.get(singleThreadExecutorName); - if (existingSingleThreadExecutor != null) { - existingSingleThreadExecutor.shutdownNow(); - executors.remove(singleThreadExecutorName); + String transactionalExecutorName = getExecutorName(true); + ExecutorService existingTransactionalExecutor = executors.get(transactionalExecutorName); + if (existingTransactionalExecutor != null) { + existingTransactionalExecutor.shutdownNow(); + executors.remove(transactionalExecutorName); } - String threadPoolExecutorName = getExecutorName(false); - ExecutorService existingThreadPoolExecutor = executors.get(threadPoolExecutorName); - if (existingThreadPoolExecutor != null) { - existingThreadPoolExecutor.shutdownNow(); - executors.remove(threadPoolExecutorName); + String nonTransactionalExecutorName = getExecutorName(false); + ExecutorService existingNonTransactionalExecutor = executors.get(nonTransactionalExecutorName); + if (existingNonTransactionalExecutor != null) { + existingNonTransactionalExecutor.shutdownNow(); + executors.remove(nonTransactionalExecutorName); } } diff --git a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java index f51c6b9f95..4aef48ea6a 100644 --- a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java +++ b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java @@ -123,18 +123,18 @@ private String getExecutorName(boolean isTransactional) { @Override public void onCatalystInstanceDestroy() { - String singleThreadExecutorName = getExecutorName(false); - ExecutorService existingSingleThreadExecutor = executors.get(singleThreadExecutorName); - if (existingSingleThreadExecutor != null) { - existingSingleThreadExecutor.shutdownNow(); - executors.remove(singleThreadExecutorName); + String transactionalExecutorName = getExecutorName(true); + ExecutorService existingTransactionalExecutor = executors.get(transactionalExecutorName); + if (existingTransactionalExecutor != null) { + existingTransactionalExecutor.shutdownNow(); + executors.remove(transactionalExecutorName); } - String threadPoolExecutorName = getExecutorName(false); - ExecutorService existingThreadPoolExecutor = executors.get(threadPoolExecutorName); - if (existingThreadPoolExecutor != null) { - existingThreadPoolExecutor.shutdownNow(); - executors.remove(threadPoolExecutorName); + String nonTransactionalExecutorName = getExecutorName(false); + ExecutorService existingNonTransactionalExecutor = executors.get(nonTransactionalExecutorName); + if (existingNonTransactionalExecutor != null) { + existingNonTransactionalExecutor.shutdownNow(); + executors.remove(nonTransactionalExecutorName); } } From b61446abdbd6048b4d4b1f08198175cf6fa4e802 Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Wed, 3 Mar 2021 20:58:56 +0900 Subject: [PATCH 06/22] fix typo --- .../io/invertase/firebase/common/UniversalFirebaseModule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java index 0777beef0c..450e83fef0 100644 --- a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java +++ b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java @@ -85,7 +85,7 @@ public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { }; }; - private String getExecutorName(Boolean isTransactional) { + private String getExecutorName(boolean isTransactional) { String moduleName = getName(); if (isTransactional == true) { return moduleName + "TransactionalExecutor"; From e8a089433eb1d85774bcb07f40e569b12afd6812 Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Fri, 5 Mar 2021 19:52:48 +0900 Subject: [PATCH 07/22] transactional executor with identifier fix typo --- .../common/UniversalFirebaseModule.java | 34 ++++++----- .../common/ReactNativeFirebaseModule.java | 61 +++++++++++-------- 2 files changed, 56 insertions(+), 39 deletions(-) diff --git a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java index 450e83fef0..19b2d3b45f 100644 --- a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java +++ b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java @@ -21,6 +21,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.RejectedExecutionHandler; @@ -85,12 +86,12 @@ public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { }; }; - private String getExecutorName(boolean isTransactional) { - String moduleName = getName(); + public String getExecutorName(boolean isTransactional) { + String name = getName(); if (isTransactional == true) { - return moduleName + "TransactionalExecutor"; + return name + "TransactionalExecutor"; } - return moduleName + "Executor"; + return name + "Executor"; } public String getName() { @@ -99,18 +100,21 @@ public String getName() { @OverridingMethodsMustInvokeSuper public void onTearDown() { - String transactionalExecutorName = getExecutorName(true); - ExecutorService existingTransactionalExecutor = executors.get(transactionalExecutorName); - if (existingTransactionalExecutor != null) { - existingTransactionalExecutor.shutdownNow(); - executors.remove(transactionalExecutorName); - } + String name = getName(); + Set existingExecutorNames = executors.keySet(); + existingExecutorNames.removeIf((executorName) -> { + return executorName.startsWith(name) == false; + }); + existingExecutorNames.forEach((executorName) -> { + removeExecutor(executorName); + }); + } - String nonTransactionalExecutorName = getExecutorName(false); - ExecutorService existingNonTransactionalExecutor = executors.get(nonTransactionalExecutorName); - if (existingNonTransactionalExecutor != null) { - existingNonTransactionalExecutor.shutdownNow(); - executors.remove(nonTransactionalExecutorName); + public void removeExecutor(String executorName) { + ExecutorService existingExecutor = executors.get(executorName); + if (existingExecutor != null) { + existingExecutor.shutdownNow(); + executors.remove(executorName); } } diff --git a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java index 4aef48ea6a..2028f12cee 100644 --- a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java +++ b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java @@ -25,6 +25,7 @@ import javax.annotation.Nonnull; import java.util.HashMap; import java.util.Map; +import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.RejectedExecutionHandler; @@ -80,15 +81,19 @@ public ReactContext getContext() { } public ExecutorService getExecutor() { - return getExecutor(false); + return getExecutor(false, ""); } public ExecutorService getTransactionalExecutor() { - return getExecutor(true); + return getExecutor(true, ""); } - private ExecutorService getExecutor(boolean isTransactional) { - String executorName = getExecutorName(isTransactional); + public ExecutorService getTransactionalExecutor(String identifier) { + return getExecutor(true, identifier); + } + + public ExecutorService getExecutor(boolean isTransactional, String identifier) { + String executorName = getExecutorName(isTransactional, identifier); ExecutorService existingExecutor = executors.get(executorName); if (existingExecutor != null) return existingExecutor; ExecutorService newExecutor = getNewExecutor(isTransactional); @@ -108,33 +113,33 @@ private ExecutorService getNewExecutor(boolean isTransactional) { private RejectedExecutionHandler executeInFallback = new RejectedExecutionHandler() { public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { - ExecutorService fallbackExecutor = getTransactionalExecutor(); + ExecutorService fallbackExecutor = getTransactionalExecutor(""); fallbackExecutor.execute(r); }; }; - private String getExecutorName(boolean isTransactional) { - String moduleName = getName(); - if (isTransactional == true) { - return moduleName + "TransactionalExecutor"; - } - return moduleName + "Executor"; - } - @Override public void onCatalystInstanceDestroy() { - String transactionalExecutorName = getExecutorName(true); - ExecutorService existingTransactionalExecutor = executors.get(transactionalExecutorName); - if (existingTransactionalExecutor != null) { - existingTransactionalExecutor.shutdownNow(); - executors.remove(transactionalExecutorName); - } + String name = getName(); + Set existingExecutorNames = executors.keySet(); + existingExecutorNames.removeIf((executorName) -> { + return executorName.startsWith(name) == false; + }); + existingExecutorNames.forEach((executorName) -> { + removeExecutor(executorName); + }); + } - String nonTransactionalExecutorName = getExecutorName(false); - ExecutorService existingNonTransactionalExecutor = executors.get(nonTransactionalExecutorName); - if (existingNonTransactionalExecutor != null) { - existingNonTransactionalExecutor.shutdownNow(); - executors.remove(nonTransactionalExecutorName); + public void onEventListenerRemove(String identifier) { + String executorName = getExecutorName(true, identifier); + removeExecutor(executorName); + } + + public void removeExecutor(String executorName) { + ExecutorService existingExecutor = executors.get(executorName); + if (existingExecutor != null) { + existingExecutor.shutdownNow(); + executors.remove(executorName); } } @@ -146,6 +151,14 @@ public Activity getActivity() { return getCurrentActivity(); } + public String getExecutorName(boolean isTransactional, String identifier) { + String name = getName(); + if (isTransactional == true) { + return name + "TransactionalExecutor" + identifier; + } + return name + "Executor" + identifier; + } + @Nonnull @Override public String getName() { From 4a10a1e158dad9c9d17d43c729cb72ff0b3f6bca Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Fri, 5 Mar 2021 20:24:36 +0900 Subject: [PATCH 08/22] rename method --- .../io/invertase/firebase/common/ReactNativeFirebaseModule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java index 2028f12cee..488c0f3f5e 100644 --- a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java +++ b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java @@ -130,7 +130,7 @@ public void onCatalystInstanceDestroy() { }); } - public void onEventListenerRemove(String identifier) { + public void removeEventListeningExecutor(String identifier) { String executorName = getExecutorName(true, identifier); removeExecutor(executorName); } From a0a7a22f62231cb674918dc218dd4b4cbce1e351 Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Fri, 5 Mar 2021 20:26:02 +0900 Subject: [PATCH 09/22] execute database event in serial --- .../database/ReactNativeFirebaseDatabaseQueryModule.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/database/android/src/reactnative/java/io/invertase/firebase/database/ReactNativeFirebaseDatabaseQueryModule.java b/packages/database/android/src/reactnative/java/io/invertase/firebase/database/ReactNativeFirebaseDatabaseQueryModule.java index e0ee9c08a6..22e0c3ef3f 100644 --- a/packages/database/android/src/reactnative/java/io/invertase/firebase/database/ReactNativeFirebaseDatabaseQueryModule.java +++ b/packages/database/android/src/reactnative/java/io/invertase/firebase/database/ReactNativeFirebaseDatabaseQueryModule.java @@ -289,7 +289,8 @@ private void handleDatabaseEvent( DataSnapshot dataSnapshot, @Nullable String previousChildName ) { - Tasks.call(getExecutor(), () -> { + final String eventRegistrationKey = registration.getString("eventRegistrationKey"); + Tasks.call(getTransactionalExecutor(eventRegistrationKey), () -> { if (eventType.equals("value")) { return snapshotToMap(dataSnapshot); } else { @@ -407,6 +408,7 @@ public void off(String queryKey, String eventRegistrationKey) { if (databaseQuery != null) { databaseQuery.removeEventListener(eventRegistrationKey); + removeEventListeningExecutor(eventRegistrationKey); if (!databaseQuery.hasListeners()) { queryMap.remove(queryKey); From 8486ff5659af9df657d0bcde7387f4065a9c19cb Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Fri, 5 Mar 2021 20:30:16 +0900 Subject: [PATCH 10/22] execute firestore event in serial --- .../ReactNativeFirebaseFirestoreCollectionModule.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java index 8461ad4cb7..c55b8cb55c 100644 --- a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java +++ b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java @@ -111,6 +111,7 @@ public void collectionOffSnapshot( if (listenerRegistration != null) { listenerRegistration.remove(); collectionSnapshotListeners.remove(listenerId); + removeEventListeningExecutor(Integer.toString(listenerId)); } } @@ -159,7 +160,7 @@ public void collectionGet( } private void sendOnSnapshotEvent(String appName, int listenerId, QuerySnapshot querySnapshot, MetadataChanges metadataChanges) { - Tasks.call(getExecutor(), () -> snapshotToWritableMap("onSnapshot", querySnapshot, metadataChanges)).addOnCompleteListener(task -> { + Tasks.call(getTransactionalExecutor(Integer.toString(listenerId)), () -> snapshotToWritableMap("onSnapshot", querySnapshot, metadataChanges)).addOnCompleteListener(task -> { if (task.isSuccessful()) { WritableMap body = Arguments.createMap(); body.putMap("snapshot", task.getResult()); From 9f6efd1c25c6a60779a2d0b5f99f9b46e418402e Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Sat, 13 Mar 2021 18:16:31 +0900 Subject: [PATCH 11/22] absctract task excutor --- .../firebase/common/TaskExecutorService.java | 108 ++++++++++++++++++ .../common/UniversalFirebaseModule.java | 69 +---------- .../common/ReactNativeFirebaseModule.java | 74 ++---------- 3 files changed, 123 insertions(+), 128 deletions(-) create mode 100644 packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java diff --git a/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java b/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java new file mode 100644 index 0000000000..139f1757c1 --- /dev/null +++ b/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java @@ -0,0 +1,108 @@ +package io.invertase.firebase.common; + +/* + * Copyright (c) 2016-present Invertase Limited & Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this library except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.RejectedExecutionHandler; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.SynchronousQueue; + +public class TaskExecutorService { + private final String name; + private final int maximumPoolSize; + private final int keepAliveSeconds; + private static Map executors = new HashMap<>(); + + TaskExecutorService( + String name, + int maximumPoolSize, + int keepAliveSeconds + ) { + this.name = name; + this.maximumPoolSize = maximumPoolSize; + this.keepAliveSeconds = keepAliveSeconds; + } + + public ExecutorService getExecutor() { + return getExecutor(false, ""); + } + + public ExecutorService getTransactionalExecutor() { + return getExecutor(true, ""); + } + + public ExecutorService getTransactionalExecutor(String identifier) { + return getExecutor(true, identifier); + } + + public ExecutorService getExecutor(boolean isTransactional, String identifier) { + String executorName = getExecutorName(isTransactional, identifier); + ExecutorService existingExecutor = executors.get(executorName); + if (existingExecutor != null) return existingExecutor; + ExecutorService newExecutor = getNewExecutor(isTransactional); + executors.put(executorName, newExecutor); + return newExecutor; + } + + private ExecutorService getNewExecutor(boolean isTransactional) { + if (isTransactional == true) { + return Executors.newSingleThreadExecutor(); + } else { + ThreadPoolExecutor threadPoolExecutor = new ThreadPoolExecutor(0, maximumPoolSize, keepAliveSeconds, TimeUnit.SECONDS, new SynchronousQueue()); + threadPoolExecutor.setRejectedExecutionHandler(executeInFallback); + return threadPoolExecutor; + } + } + + private RejectedExecutionHandler executeInFallback = new RejectedExecutionHandler() { + public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { + ExecutorService fallbackExecutor = getTransactionalExecutor(); + fallbackExecutor.execute(r); + }; + }; + + public String getExecutorName(boolean isTransactional, String identifier) { + if (isTransactional == true) { + return name + "TransactionalExecutor" + identifier; + } + return name + "Executor" + identifier; + } + + public void shutdown() { + Set existingExecutorNames = executors.keySet(); + existingExecutorNames.removeIf((executorName) -> { + return executorName.startsWith(name) == false; + }); + existingExecutorNames.forEach((executorName) -> { + removeExecutor(executorName); + }); + } + + public void removeExecutor(String executorName) { + ExecutorService existingExecutor = executors.get(executorName); + if (existingExecutor != null) { + existingExecutor.shutdownNow(); + executors.remove(executorName); + } + } +} diff --git a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java index 19b2d3b45f..36eb4e1cf7 100644 --- a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java +++ b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java @@ -18,23 +18,16 @@ */ import android.content.Context; +import io.invertase.firebase.common.TaskExecutorService; import java.util.HashMap; import java.util.Map; -import java.util.Set; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.RejectedExecutionHandler; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.SynchronousQueue; import javax.annotation.OverridingMethodsMustInvokeSuper; public class UniversalFirebaseModule { - private static final int MAXIMUM_POOL_SIZE = 20; - private static final int KEEP_ALIVE_SECONDS = 3; - private static Map executors = new HashMap<>(); + private final TaskExecutorService executorService; private final Context context; private final String serviceName; @@ -42,6 +35,7 @@ public class UniversalFirebaseModule { protected UniversalFirebaseModule(Context context, String serviceName) { this.context = context; this.serviceName = serviceName; + this.executorService = new TaskExecutorService(getName(), 20, 3); // TODO: tunable pool sizing } public Context getContext() { @@ -53,45 +47,7 @@ public Context getApplicationContext() { } protected ExecutorService getExecutor() { - return getExecutor(false); - } - - protected ExecutorService getTransactionalExecutor() { - return getExecutor(true); - } - - private ExecutorService getExecutor(boolean isTransactional) { - String executorName = getExecutorName(isTransactional); - ExecutorService existingExecutor = executors.get(executorName); - if (existingExecutor != null) return existingExecutor; - ExecutorService newExecutor = getNewExecutor(isTransactional); - executors.put(executorName, newExecutor); - return newExecutor; - } - - private ExecutorService getNewExecutor(boolean isTransactional) { - if (isTransactional == true) { - return Executors.newSingleThreadExecutor(); - } else { - ThreadPoolExecutor threadPoolExecutor = new ThreadPoolExecutor(0, MAXIMUM_POOL_SIZE, KEEP_ALIVE_SECONDS, TimeUnit.SECONDS, new SynchronousQueue()); - threadPoolExecutor.setRejectedExecutionHandler(executeInFallback); - return threadPoolExecutor; - } - } - - private RejectedExecutionHandler executeInFallback = new RejectedExecutionHandler() { - public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { - ExecutorService fallbackExecutor = getTransactionalExecutor(); - fallbackExecutor.execute(r); - }; - }; - - public String getExecutorName(boolean isTransactional) { - String name = getName(); - if (isTransactional == true) { - return name + "TransactionalExecutor"; - } - return name + "Executor"; + return executorService.getExecutor(); } public String getName() { @@ -100,22 +56,7 @@ public String getName() { @OverridingMethodsMustInvokeSuper public void onTearDown() { - String name = getName(); - Set existingExecutorNames = executors.keySet(); - existingExecutorNames.removeIf((executorName) -> { - return executorName.startsWith(name) == false; - }); - existingExecutorNames.forEach((executorName) -> { - removeExecutor(executorName); - }); - } - - public void removeExecutor(String executorName) { - ExecutorService existingExecutor = executors.get(executorName); - if (existingExecutor != null) { - existingExecutor.shutdownNow(); - executors.remove(executorName); - } + executorService.shutdown(); } public Map getConstants() { diff --git a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java index 488c0f3f5e..f279605aa7 100644 --- a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java +++ b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java @@ -21,22 +21,16 @@ import android.content.Context; import com.facebook.react.bridge.*; import io.invertase.firebase.interfaces.ContextProvider; +import io.invertase.firebase.common.TaskExecutorService; import javax.annotation.Nonnull; import java.util.HashMap; import java.util.Map; -import java.util.Set; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.RejectedExecutionHandler; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.SynchronousQueue; public class ReactNativeFirebaseModule extends ReactContextBaseJavaModule implements ContextProvider { - private static final int MAXIMUM_POOL_SIZE = 20; - private static final int KEEP_ALIVE_SECONDS = 3; - private static Map executors = new HashMap<>(); + private final TaskExecutorService executorService; + private String moduleName; public ReactNativeFirebaseModule( @@ -45,6 +39,7 @@ public ReactNativeFirebaseModule( ) { super(reactContext); this.moduleName = moduleName; + this.executorService = new TaskExecutorService(getName(), 20, 3); // TODO: tunable pool sizing } public static void rejectPromiseWithExceptionMap(Promise promise, Exception exception) { @@ -81,66 +76,25 @@ public ReactContext getContext() { } public ExecutorService getExecutor() { - return getExecutor(false, ""); + return executorService.getExecutor(); } public ExecutorService getTransactionalExecutor() { - return getExecutor(true, ""); + return executorService.getTransactionalExecutor(); } public ExecutorService getTransactionalExecutor(String identifier) { - return getExecutor(true, identifier); - } - - public ExecutorService getExecutor(boolean isTransactional, String identifier) { - String executorName = getExecutorName(isTransactional, identifier); - ExecutorService existingExecutor = executors.get(executorName); - if (existingExecutor != null) return existingExecutor; - ExecutorService newExecutor = getNewExecutor(isTransactional); - executors.put(executorName, newExecutor); - return newExecutor; - } - - private ExecutorService getNewExecutor(boolean isTransactional) { - if (isTransactional == true) { - return Executors.newSingleThreadExecutor(); - } else { - ThreadPoolExecutor threadPoolExecutor = new ThreadPoolExecutor(0, MAXIMUM_POOL_SIZE, KEEP_ALIVE_SECONDS, TimeUnit.SECONDS, new SynchronousQueue()); - threadPoolExecutor.setRejectedExecutionHandler(executeInFallback); - return threadPoolExecutor; - } + return executorService.getTransactionalExecutor(identifier); } - private RejectedExecutionHandler executeInFallback = new RejectedExecutionHandler() { - public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { - ExecutorService fallbackExecutor = getTransactionalExecutor(""); - fallbackExecutor.execute(r); - }; - }; - @Override public void onCatalystInstanceDestroy() { - String name = getName(); - Set existingExecutorNames = executors.keySet(); - existingExecutorNames.removeIf((executorName) -> { - return executorName.startsWith(name) == false; - }); - existingExecutorNames.forEach((executorName) -> { - removeExecutor(executorName); - }); + executorService.shutdown(); } public void removeEventListeningExecutor(String identifier) { - String executorName = getExecutorName(true, identifier); - removeExecutor(executorName); - } - - public void removeExecutor(String executorName) { - ExecutorService existingExecutor = executors.get(executorName); - if (existingExecutor != null) { - existingExecutor.shutdownNow(); - executors.remove(executorName); - } + String executorName = executorService.getExecutorName(true, identifier); + executorService.removeExecutor(executorName); } public Context getApplicationContext() { @@ -151,14 +105,6 @@ public Activity getActivity() { return getCurrentActivity(); } - public String getExecutorName(boolean isTransactional, String identifier) { - String name = getName(); - if (isTransactional == true) { - return name + "TransactionalExecutor" + identifier; - } - return name + "Executor" + identifier; - } - @Nonnull @Override public String getName() { From 10fcab9b696e6eab8cbd98cb663d30a58bad7c68 Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Sat, 13 Mar 2021 18:29:40 +0900 Subject: [PATCH 12/22] do not re-excute rejected task while shutdown --- .../java/io/invertase/firebase/common/TaskExecutorService.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java b/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java index 139f1757c1..ea31d40209 100644 --- a/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java +++ b/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java @@ -76,6 +76,9 @@ private ExecutorService getNewExecutor(boolean isTransactional) { private RejectedExecutionHandler executeInFallback = new RejectedExecutionHandler() { public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { + if (executor.isShutdown() || executor.isTerminated() || executor.isTerminating()) { + return; + } ExecutorService fallbackExecutor = getTransactionalExecutor(); fallbackExecutor.execute(r); }; From 8bdabcd120bc962bd0da892ec38f9804139b63ab Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Sat, 13 Mar 2021 20:05:27 +0900 Subject: [PATCH 13/22] tunable thread pool executor --- .../firebase/common/TaskExecutorService.java | 18 +++++++++++------- .../common/UniversalFirebaseModule.java | 2 +- .../common/ReactNativeFirebaseJSON.java | 5 +++++ .../common/ReactNativeFirebaseModule.java | 2 +- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java b/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java index ea31d40209..ad7fab01dc 100644 --- a/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java +++ b/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java @@ -17,6 +17,8 @@ * */ +import io.invertase.firebase.common.ReactNativeFirebaseJSON; + import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -28,19 +30,19 @@ import java.util.concurrent.SynchronousQueue; public class TaskExecutorService { + private static final String MAXIMUM_POOL_SIZE_KEY = "android_task_executor_maximum_pool_size"; + private static final String KEEP_ALIVE_SECONDS_KEY = "android_task_executor_keep_alive_seconds"; + private final String name; private final int maximumPoolSize; private final int keepAliveSeconds; private static Map executors = new HashMap<>(); - TaskExecutorService( - String name, - int maximumPoolSize, - int keepAliveSeconds - ) { + TaskExecutorService(String name) { this.name = name; - this.maximumPoolSize = maximumPoolSize; - this.keepAliveSeconds = keepAliveSeconds; + ReactNativeFirebaseJSON json = ReactNativeFirebaseJSON.getSharedInstance(); + this.maximumPoolSize = json.getIntValue(MAXIMUM_POOL_SIZE_KEY, 1); + this.keepAliveSeconds = json.getIntValue(KEEP_ALIVE_SECONDS_KEY, 3); } public ExecutorService getExecutor() { @@ -67,6 +69,8 @@ public ExecutorService getExecutor(boolean isTransactional, String identifier) { private ExecutorService getNewExecutor(boolean isTransactional) { if (isTransactional == true) { return Executors.newSingleThreadExecutor(); + } else if (maximumPoolSize == 1) { + return Executors.newSingleThreadExecutor(); } else { ThreadPoolExecutor threadPoolExecutor = new ThreadPoolExecutor(0, maximumPoolSize, keepAliveSeconds, TimeUnit.SECONDS, new SynchronousQueue()); threadPoolExecutor.setRejectedExecutionHandler(executeInFallback); diff --git a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java index 36eb4e1cf7..1ac80d4291 100644 --- a/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java +++ b/packages/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebaseModule.java @@ -35,7 +35,7 @@ public class UniversalFirebaseModule { protected UniversalFirebaseModule(Context context, String serviceName) { this.context = context; this.serviceName = serviceName; - this.executorService = new TaskExecutorService(getName(), 20, 3); // TODO: tunable pool sizing + this.executorService = new TaskExecutorService(getName()); } public Context getContext() { diff --git a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseJSON.java b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseJSON.java index 1d9c96f725..b45ad2f01b 100644 --- a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseJSON.java +++ b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseJSON.java @@ -53,6 +53,11 @@ public boolean getBooleanValue(String key, boolean defaultValue) { return jsonObject.optBoolean(key, defaultValue); } + public int getIntValue(String key, int defaultValue) { + if (jsonObject == null) return defaultValue; + return jsonObject.optInt(key, defaultValue); + } + public long getLongValue(String key, long defaultValue) { if (jsonObject == null) return defaultValue; return jsonObject.optLong(key, defaultValue); diff --git a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java index f279605aa7..f3f61983ec 100644 --- a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java +++ b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java @@ -39,7 +39,7 @@ public ReactNativeFirebaseModule( ) { super(reactContext); this.moduleName = moduleName; - this.executorService = new TaskExecutorService(getName(), 20, 3); // TODO: tunable pool sizing + this.executorService = new TaskExecutorService(getName()); } public static void rejectPromiseWithExceptionMap(Promise promise, Exception exception) { From 279688ac1620e4b956fcdedd4f071721f13fcc51 Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Sat, 13 Mar 2021 21:08:12 +0900 Subject: [PATCH 14/22] add documentation --- docs/index.md | 19 +++++++++++++++++++ packages/app/firebase-schema.json | 8 ++++++++ 2 files changed, 27 insertions(+) diff --git a/docs/index.md b/docs/index.md index 660a1a7380..e40779f33c 100644 --- a/docs/index.md +++ b/docs/index.md @@ -227,6 +227,25 @@ setting present in `/android/gradle.properties`: org.gradle.jvmargs=-Xmx2048m -XX:MaxPermSize=512m -XX:+HeapDumpOnOutOfMemoryError -Dfile.encoding=UTF-8 ``` +### Android Performance + +On Android, React Native Firebase uses [thread pool executor](https://developer.android.com/reference/java/util/concurrent/ThreadPoolExecutor) to provide improved performance and managed resources. +To increase throughput, you can tune the thread pool executor via `firebase.json` file within the root of your project: + +```json +// /firebase.json +{ + "react-native": { + // Maximum pool size of ThreadPoolExecutor. Defaults to `1`. + // Larger value typically improve performance when executing large numbers of asynchronous tasks, e.g. Firestore queries. + "android_task_executor_maximum_pool_size": 1, + // Keep-alive time of ThreadPoolExecutor, in seconds. Defaults to `3` + // Excess threads in the pool executor will be terminated if they have been idle for more than the keep-alive time. + "android_task_executor_keep_alive_seconds": 3, + } +} +``` + ### Allow iOS Static Frameworks If you are using Static Frameworks on iOS, you need to manually enable this for the project. To enable Static Framework diff --git a/packages/app/firebase-schema.json b/packages/app/firebase-schema.json index 293cb9087f..e9e4a49669 100644 --- a/packages/app/firebase-schema.json +++ b/packages/app/firebase-schema.json @@ -65,6 +65,14 @@ "perf_auto_collection_enabled": { "description": "Disable or enable auto collection of performance monitoring data collection.\n This is useful for opt-in-first data flows, for example when dealing with GDPR compliance.\nThis can be overridden in JavaScript.", "type": "boolean" + }, + "android_task_executor_maximum_pool_size" : { + "description": "Maximum pool size of ThreadPoolExecutor used by RNFirebase for Android. Defaults to `1`.\n Larger value typically improve performance when executing large numbers of asynchronous tasks, e.g. Firestore queries.", + "type": "number" + }, + "android_task_executor_keep_alive_seconds" : { + "description": "Keep-alive time of ThreadPoolExecutor used by RNFirebase for Android, in seconds. Defaults to `3`.\n Excess threads in the pool executor will be terminated if they have been idle for more than the keep-alive time.", + "type": "number" } } } From 24cd9a9b21e7f46c5921261a434542bacc733cbf Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Sat, 13 Mar 2021 21:08:26 +0900 Subject: [PATCH 15/22] tests configuration --- tests/firebase.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/firebase.json b/tests/firebase.json index 776fde2a29..b3803a17b6 100644 --- a/tests/firebase.json +++ b/tests/firebase.json @@ -20,6 +20,9 @@ "perf_auto_collection_enabled": true, + "android_task_executor_maximum_pool_size": 10, + "android_task_executor_keep_alive_seconds": 3, + "TODO_in_app_messaging_auto_collection_enabled": true, "TODO_database_persistence_enabled": true, "TODO_firestore_persistence_enabled": true, From efed5492aa78f028c7402150ad19959ac6d0d8d3 Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Sat, 13 Mar 2021 22:38:00 +0900 Subject: [PATCH 16/22] disable identified executor when maximum pool size is zero --- .../io/invertase/firebase/common/TaskExecutorService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java b/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java index ad7fab01dc..62c51c3559 100644 --- a/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java +++ b/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java @@ -46,7 +46,8 @@ public class TaskExecutorService { } public ExecutorService getExecutor() { - return getExecutor(false, ""); + boolean isTransactional = maximumPoolSize <= 1; + return getExecutor(isTransactional, ""); } public ExecutorService getTransactionalExecutor() { @@ -54,7 +55,8 @@ public ExecutorService getTransactionalExecutor() { } public ExecutorService getTransactionalExecutor(String identifier) { - return getExecutor(true, identifier); + String executorIdentifier = maximumPoolSize != 0 ? identifier : ""; + return getExecutor(true, executorIdentifier); } public ExecutorService getExecutor(boolean isTransactional, String identifier) { @@ -69,8 +71,6 @@ public ExecutorService getExecutor(boolean isTransactional, String identifier) { private ExecutorService getNewExecutor(boolean isTransactional) { if (isTransactional == true) { return Executors.newSingleThreadExecutor(); - } else if (maximumPoolSize == 1) { - return Executors.newSingleThreadExecutor(); } else { ThreadPoolExecutor threadPoolExecutor = new ThreadPoolExecutor(0, maximumPoolSize, keepAliveSeconds, TimeUnit.SECONDS, new SynchronousQueue()); threadPoolExecutor.setRejectedExecutionHandler(executeInFallback); From 4ed1cb64cdea45fad66bf9c1be25b19b6551e4d6 Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Sat, 13 Mar 2021 23:08:46 +0900 Subject: [PATCH 17/22] update document --- docs/index.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/index.md b/docs/index.md index e40779f33c..e7e40caedc 100644 --- a/docs/index.md +++ b/docs/index.md @@ -238,9 +238,11 @@ To increase throughput, you can tune the thread pool executor via `firebase.json "react-native": { // Maximum pool size of ThreadPoolExecutor. Defaults to `1`. // Larger value typically improve performance when executing large numbers of asynchronous tasks, e.g. Firestore queries. + // Setting this value to `0` completely disables the pooled executor and all tasks execute in serial per module. "android_task_executor_maximum_pool_size": 1, // Keep-alive time of ThreadPoolExecutor, in seconds. Defaults to `3` // Excess threads in the pool executor will be terminated if they have been idle for more than the keep-alive time. + // This value doesn't have any effect when the maximum pool size is lower than 2. "android_task_executor_keep_alive_seconds": 3, } } From e995f280ad009af49a12c784996121511b7a25cf Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Sun, 14 Mar 2021 17:27:55 +0900 Subject: [PATCH 18/22] cleaner documentation --- docs/index.md | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/docs/index.md b/docs/index.md index e7e40caedc..3e88479a77 100644 --- a/docs/index.md +++ b/docs/index.md @@ -229,25 +229,24 @@ org.gradle.jvmargs=-Xmx2048m -XX:MaxPermSize=512m -XX:+HeapDumpOnOutOfMemoryErro ### Android Performance -On Android, React Native Firebase uses [thread pool executor](https://developer.android.com/reference/java/util/concurrent/ThreadPoolExecutor) to provide improved performance and managed resources. +On Android, React Native Firebase uses [ThreadPoolExecutor](https://developer.android.com/reference/java/util/concurrent/ThreadPoolExecutor) to provide improved performance and managed resources. To increase throughput, you can tune the thread pool executor via `firebase.json` file within the root of your project: ```json // /firebase.json { "react-native": { - // Maximum pool size of ThreadPoolExecutor. Defaults to `1`. - // Larger value typically improve performance when executing large numbers of asynchronous tasks, e.g. Firestore queries. - // Setting this value to `0` completely disables the pooled executor and all tasks execute in serial per module. - "android_task_executor_maximum_pool_size": 1, - // Keep-alive time of ThreadPoolExecutor, in seconds. Defaults to `3` - // Excess threads in the pool executor will be terminated if they have been idle for more than the keep-alive time. - // This value doesn't have any effect when the maximum pool size is lower than 2. + "android_task_executor_maximum_pool_size": 10, "android_task_executor_keep_alive_seconds": 3, } } ``` +| Key | Description | +| ------------ | ----------------------------------------------- | +| `android_task_executor_maximum_pool_size` | Maximum pool size of ThreadPoolExecutor. Defaults to `1`. Larger value typically improve performance when executing large numbers of asynchronous tasks, e.g. Firestore queries. Setting this value to `0` completely disables the pooled executor and all tasks execute in serial per module. | +| `android_task_executor_keep_alive_seconds` | Keep-alive time of ThreadPoolExecutor, in seconds. Defaults to `3`. Excess threads in the pool executor will be terminated if they have been idle for more than the keep-alive time. This value doesn't have any effect when the maximum pool size is lower than `2`. | + ### Allow iOS Static Frameworks If you are using Static Frameworks on iOS, you need to manually enable this for the project. To enable Static Framework From e318a82cd7b3cc1114a31f4fde627c16409e8117 Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Sun, 14 Mar 2021 17:31:05 +0900 Subject: [PATCH 19/22] fix typo --- docs/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/index.md b/docs/index.md index 3e88479a77..fd6c1c9fe9 100644 --- a/docs/index.md +++ b/docs/index.md @@ -229,7 +229,7 @@ org.gradle.jvmargs=-Xmx2048m -XX:MaxPermSize=512m -XX:+HeapDumpOnOutOfMemoryErro ### Android Performance -On Android, React Native Firebase uses [ThreadPoolExecutor](https://developer.android.com/reference/java/util/concurrent/ThreadPoolExecutor) to provide improved performance and managed resources. +On Android, React Native Firebase uses [thread pool executor](https://developer.android.com/reference/java/util/concurrent/ThreadPoolExecutor) to provide improved performance and managed resources. To increase throughput, you can tune the thread pool executor via `firebase.json` file within the root of your project: ```json From 911582cefe2e7b3165ab6875c55b9cfb51d15f25 Mon Sep 17 00:00:00 2001 From: Mike Hardy Date: Thu, 1 Apr 2021 19:46:11 -0500 Subject: [PATCH 20/22] Apply suggestions from code review The tiniest of grammar nitpicks --- docs/index.md | 2 +- packages/app/firebase-schema.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/index.md b/docs/index.md index 37b7218d7c..462b0cf107 100644 --- a/docs/index.md +++ b/docs/index.md @@ -243,7 +243,7 @@ To increase throughput, you can tune the thread pool executor via `firebase.json | Key | Description | | ------------ | ----------------------------------------------- | -| `android_task_executor_maximum_pool_size` | Maximum pool size of ThreadPoolExecutor. Defaults to `1`. Larger value typically improve performance when executing large numbers of asynchronous tasks, e.g. Firestore queries. Setting this value to `0` completely disables the pooled executor and all tasks execute in serial per module. | +| `android_task_executor_maximum_pool_size` | Maximum pool size of ThreadPoolExecutor. Defaults to `1`. Larger values typically improve performance when executing large numbers of asynchronous tasks, e.g. Firestore queries. Setting this value to `0` completely disables the pooled executor and all tasks execute in serial per module. | | `android_task_executor_keep_alive_seconds` | Keep-alive time of ThreadPoolExecutor, in seconds. Defaults to `3`. Excess threads in the pool executor will be terminated if they have been idle for more than the keep-alive time. This value doesn't have any effect when the maximum pool size is lower than `2`. | ### Allow iOS Static Frameworks diff --git a/packages/app/firebase-schema.json b/packages/app/firebase-schema.json index e9e4a49669..e9d1f9c01b 100644 --- a/packages/app/firebase-schema.json +++ b/packages/app/firebase-schema.json @@ -67,7 +67,7 @@ "type": "boolean" }, "android_task_executor_maximum_pool_size" : { - "description": "Maximum pool size of ThreadPoolExecutor used by RNFirebase for Android. Defaults to `1`.\n Larger value typically improve performance when executing large numbers of asynchronous tasks, e.g. Firestore queries.", + "description": "Maximum pool size of ThreadPoolExecutor used by RNFirebase for Android. Defaults to `1`.\n Larger values typically improve performance when executing large numbers of asynchronous tasks, e.g. Firestore queries.",d "type": "number" }, "android_task_executor_keep_alive_seconds" : { From 50a4d98316980e6cb6cc4e95e88e6cf7704fc86e Mon Sep 17 00:00:00 2001 From: Mike Hardy Date: Thu, 1 Apr 2021 20:12:44 -0500 Subject: [PATCH 21/22] Update packages/app/firebase-schema.json I inadvertently broke the schema file with a mis-type of the letter `d` in the text box also fixup tiny spacing issue --- packages/app/firebase-schema.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/app/firebase-schema.json b/packages/app/firebase-schema.json index e9d1f9c01b..899a54be66 100644 --- a/packages/app/firebase-schema.json +++ b/packages/app/firebase-schema.json @@ -66,11 +66,11 @@ "description": "Disable or enable auto collection of performance monitoring data collection.\n This is useful for opt-in-first data flows, for example when dealing with GDPR compliance.\nThis can be overridden in JavaScript.", "type": "boolean" }, - "android_task_executor_maximum_pool_size" : { - "description": "Maximum pool size of ThreadPoolExecutor used by RNFirebase for Android. Defaults to `1`.\n Larger values typically improve performance when executing large numbers of asynchronous tasks, e.g. Firestore queries.",d + "android_task_executor_maximum_pool_size": { + "description": "Maximum pool size of ThreadPoolExecutor used by RNFirebase for Android. Defaults to `1`.\n Larger values typically improve performance when executing large numbers of asynchronous tasks, e.g. Firestore queries.", "type": "number" }, - "android_task_executor_keep_alive_seconds" : { + "android_task_executor_keep_alive_seconds": { "description": "Keep-alive time of ThreadPoolExecutor used by RNFirebase for Android, in seconds. Defaults to `3`.\n Excess threads in the pool executor will be terminated if they have been idle for more than the keep-alive time.", "type": "number" } From 50e690371e4bb02667bba01735275e33b31936bf Mon Sep 17 00:00:00 2001 From: Minsik Kim Date: Sat, 3 Apr 2021 21:22:27 +0900 Subject: [PATCH 22/22] Avoid race condition in executors fix typo --- .../firebase/common/TaskExecutorService.java | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java b/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java index 62c51c3559..ef7e935421 100644 --- a/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java +++ b/packages/app/android/src/main/java/io/invertase/firebase/common/TaskExecutorService.java @@ -61,11 +61,15 @@ public ExecutorService getTransactionalExecutor(String identifier) { public ExecutorService getExecutor(boolean isTransactional, String identifier) { String executorName = getExecutorName(isTransactional, identifier); - ExecutorService existingExecutor = executors.get(executorName); - if (existingExecutor != null) return existingExecutor; - ExecutorService newExecutor = getNewExecutor(isTransactional); - executors.put(executorName, newExecutor); - return newExecutor; + synchronized(executors) { + ExecutorService existingExecutor = executors.get(executorName); + if (existingExecutor == null) { + ExecutorService newExecutor = getNewExecutor(isTransactional); + executors.put(executorName, newExecutor); + return newExecutor; + } + return existingExecutor; + } } private ExecutorService getNewExecutor(boolean isTransactional) { @@ -106,10 +110,12 @@ public void shutdown() { } public void removeExecutor(String executorName) { - ExecutorService existingExecutor = executors.get(executorName); - if (existingExecutor != null) { - existingExecutor.shutdownNow(); - executors.remove(executorName); + synchronized(executors) { + ExecutorService existingExecutor = executors.get(executorName); + if (existingExecutor != null) { + existingExecutor.shutdownNow(); + executors.remove(executorName); + } } } }