From ed07741074ca01f19048a3141096ee3ed36d894a Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 5 Feb 2025 21:00:15 -0300 Subject: [PATCH 1/5] Avoid destruction of factory before initialization --- .../android/client/SplitFactoryHelper.java | 41 +++++++++++++------ .../android/client/SplitFactoryImpl.java | 8 +++- .../android/client/SplitFactoryHelperTest.kt | 11 ++++- 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/main/java/io/split/android/client/SplitFactoryHelper.java b/src/main/java/io/split/android/client/SplitFactoryHelper.java index a0649aba1..be33b61c3 100644 --- a/src/main/java/io/split/android/client/SplitFactoryHelper.java +++ b/src/main/java/io/split/android/client/SplitFactoryHelper.java @@ -20,6 +20,7 @@ import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReentrantLock; import io.split.android.client.common.CompressionUtilProvider; import io.split.android.client.events.EventsManagerCoordinator; @@ -485,6 +486,7 @@ static class Initializer implements Runnable { private final RolloutCacheManager mRolloutCacheManager; private final SplitTaskExecutionListener mListener; + private final ReentrantLock mInitLock; Initializer( String apiToken, @@ -497,23 +499,28 @@ static class Initializer implements Runnable { SplitSingleThreadTaskExecutor splitSingleThreadTaskExecutor, SplitStorageContainer storageContainer, SyncManager syncManager, - SplitLifecycleManager lifecycleManager) { + SplitLifecycleManager lifecycleManager, + ReentrantLock initLock) { this(new RolloutCacheManagerImpl(config, storageContainer, splitTaskFactory.createCleanUpDatabaseTask(System.currentTimeMillis() / 1000), splitTaskFactory.createEncryptionMigrationTask(apiToken, splitDatabase, config.encryptionEnabled(), splitCipher)), - new Listener(eventsManagerCoordinator, splitTaskExecutor, splitSingleThreadTaskExecutor, syncManager, lifecycleManager)); + new Listener(eventsManagerCoordinator, splitTaskExecutor, splitSingleThreadTaskExecutor, syncManager, lifecycleManager, initLock), + initLock); } @VisibleForTesting - Initializer(RolloutCacheManager rolloutCacheManager, SplitTaskExecutionListener listener) { + Initializer(RolloutCacheManager rolloutCacheManager, SplitTaskExecutionListener listener, ReentrantLock initLock) { mRolloutCacheManager = rolloutCacheManager; mListener = listener; + mInitLock = initLock; } @Override public void run() { + Logger.v("Running SDK initializer"); + mInitLock.lock(); mRolloutCacheManager.validateCache(mListener); } @@ -524,30 +531,38 @@ static class Listener implements SplitTaskExecutionListener { private final SplitSingleThreadTaskExecutor mSplitSingleThreadTaskExecutor; private final SyncManager mSyncManager; private final SplitLifecycleManager mLifecycleManager; + private final ReentrantLock mInitLock; Listener(EventsManagerCoordinator eventsManagerCoordinator, SplitTaskExecutor splitTaskExecutor, SplitSingleThreadTaskExecutor splitSingleThreadTaskExecutor, SyncManager syncManager, - SplitLifecycleManager lifecycleManager) { + SplitLifecycleManager lifecycleManager, + ReentrantLock initLock) { mEventsManagerCoordinator = eventsManagerCoordinator; mSplitTaskExecutor = splitTaskExecutor; mSplitSingleThreadTaskExecutor = splitSingleThreadTaskExecutor; mSyncManager = syncManager; mLifecycleManager = lifecycleManager; + mInitLock = initLock; } @Override public void taskExecuted(@NonNull SplitTaskExecutionInfo taskInfo) { - mEventsManagerCoordinator.notifyInternalEvent(SplitInternalEvent.ENCRYPTION_MIGRATION_DONE); - - mSplitTaskExecutor.resume(); - mSplitSingleThreadTaskExecutor.resume(); - - mSyncManager.start(); - mLifecycleManager.register(mSyncManager); - - Logger.i("Android SDK initialized!"); + try { + mEventsManagerCoordinator.notifyInternalEvent(SplitInternalEvent.ENCRYPTION_MIGRATION_DONE); + + mSplitTaskExecutor.resume(); + mSplitSingleThreadTaskExecutor.resume(); + + mSyncManager.start(); + mLifecycleManager.register(mSyncManager); + Logger.i("Android SDK initialized!"); + } catch (Exception e) { + Logger.e("Error initializing Android SDK", e); + } finally { + mInitLock.unlock(); + } } } } diff --git a/src/main/java/io/split/android/client/SplitFactoryImpl.java b/src/main/java/io/split/android/client/SplitFactoryImpl.java index 32ea3963c..14454b34d 100644 --- a/src/main/java/io/split/android/client/SplitFactoryImpl.java +++ b/src/main/java/io/split/android/client/SplitFactoryImpl.java @@ -13,6 +13,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.locks.ReentrantLock; import io.split.android.android_client.BuildConfig; import io.split.android.client.api.Key; @@ -83,6 +84,7 @@ public class SplitFactoryImpl implements SplitFactory { private final SplitStorageContainer mStorageContainer; private final SplitClientContainer mClientContainer; private final UserConsentManager mUserConsentManager; + private final ReentrantLock mInitLock = new ReentrantLock(); public SplitFactoryImpl(@NonNull String apiToken, @NonNull Key key, @NonNull SplitClientConfig config, @NonNull Context context) throws URISyntaxException { @@ -273,6 +275,7 @@ private SplitFactoryImpl(@NonNull String apiToken, @NonNull Key key, @NonNull Sp eventsTracker, flagSetsFilter); mDestroyer = new Runnable() { public void run() { + mInitLock.lock(); Logger.w("Shutdown called for split"); try { mStorageContainer.getTelemetryStorage().recordSessionLength(System.currentTimeMillis() - initializationStartTime); @@ -303,6 +306,8 @@ public void run() { Logger.e(e, "We could not shutdown split"); } finally { mIsTerminated = true; + Logger.d("SplitFactory has been destroyed"); + mInitLock.unlock(); } } }; @@ -325,7 +330,8 @@ public void run() { splitSingleThreadTaskExecutor, mStorageContainer, mSyncManager, - mLifecycleManager); + mLifecycleManager, + mInitLock); if (config.shouldRecordTelemetry()) { int activeFactoriesCount = mFactoryMonitor.count(mApiKey); diff --git a/src/test/java/io/split/android/client/SplitFactoryHelperTest.kt b/src/test/java/io/split/android/client/SplitFactoryHelperTest.kt index 7ff5e004b..335173c5d 100644 --- a/src/test/java/io/split/android/client/SplitFactoryHelperTest.kt +++ b/src/test/java/io/split/android/client/SplitFactoryHelperTest.kt @@ -25,6 +25,7 @@ import org.mockito.Mockito.`when` import org.mockito.MockitoAnnotations import java.io.File import java.lang.IllegalArgumentException +import java.util.concurrent.locks.ReentrantLock class SplitFactoryHelperTest { @@ -141,15 +142,18 @@ class SplitFactoryHelperTest { fun `Initializer test`() { val rolloutCacheManager = mock(RolloutCacheManager::class.java) val splitTaskExecutionListener = mock(SplitTaskExecutionListener::class.java) + val initLock = mock(ReentrantLock::class.java) val initializer = SplitFactoryHelper.Initializer( rolloutCacheManager, - splitTaskExecutionListener + splitTaskExecutionListener, + initLock ) initializer.run() verify(rolloutCacheManager).validateCache(splitTaskExecutionListener) + verify(initLock).lock() } @Test @@ -159,13 +163,15 @@ class SplitFactoryHelperTest { val singleThreadTaskExecutor = mock(SplitSingleThreadTaskExecutor::class.java) val syncManager = mock(SyncManager::class.java) val lifecycleManager = mock(SplitLifecycleManager::class.java) + val initLock = mock(ReentrantLock::class.java) val listener = Listener( eventsManagerCoordinator, taskExecutor, singleThreadTaskExecutor, syncManager, - lifecycleManager + lifecycleManager, + initLock ) listener.taskExecuted(SplitTaskExecutionInfo.success(SplitTaskType.GENERIC_TASK)) @@ -175,5 +181,6 @@ class SplitFactoryHelperTest { verify(singleThreadTaskExecutor).resume() verify(syncManager).start() verify(lifecycleManager).register(syncManager) + verify(initLock).unlock() } } From a6732c4c18f32df8df3c82d51adc70b03a06d359 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 6 Feb 2025 09:28:50 -0300 Subject: [PATCH 2/5] Factory shutdown enhancements --- .../android/client/SplitFactoryImpl.java | 78 ++++++++++++------- 1 file changed, 50 insertions(+), 28 deletions(-) diff --git a/src/main/java/io/split/android/client/SplitFactoryImpl.java b/src/main/java/io/split/android/client/SplitFactoryImpl.java index 14454b34d..c444062ef 100644 --- a/src/main/java/io/split/android/client/SplitFactoryImpl.java +++ b/src/main/java/io/split/android/client/SplitFactoryImpl.java @@ -11,8 +11,11 @@ import java.util.List; import java.util.Map; import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; import io.split.android.android_client.BuildConfig; @@ -276,37 +279,41 @@ private SplitFactoryImpl(@NonNull String apiToken, @NonNull Key key, @NonNull Sp mDestroyer = new Runnable() { public void run() { mInitLock.lock(); - Logger.w("Shutdown called for split"); try { - mStorageContainer.getTelemetryStorage().recordSessionLength(System.currentTimeMillis() - initializationStartTime); - telemetrySynchronizer.flush(); - telemetrySynchronizer.destroy(); - Logger.d("Successful shutdown of telemetry"); - impressionsLoggingTaskExecutor.shutdown(); - impressionsObserverExecutor.shutdown(); - Logger.d("Successful shutdown of impressions logging executor"); - mSyncManager.stop(); - Logger.d("Flushing impressions and events"); - mLifecycleManager.destroy(); - Logger.d("Successful shutdown of lifecycle manager"); - mFactoryMonitor.remove(mApiKey); - Logger.d("Successful shutdown of segment fetchers"); - customerImpressionListener.close(); - Logger.d("Successful shutdown of ImpressionListener"); - defaultHttpClient.close(); - Logger.d("Successful shutdown of httpclient"); - mManager.destroy(); - Logger.d("Successful shutdown of manager"); - splitTaskExecutor.stop(); - splitSingleThreadTaskExecutor.stop(); - Logger.d("Successful shutdown of task executor"); - mStorageContainer.getAttributesStorageContainer().destroy(); - Logger.d("Successful shutdown of attributes storage"); + if (mClientContainer.getAll().isEmpty()) { + Logger.w("Shutdown called for split"); + mStorageContainer.getTelemetryStorage().recordSessionLength(System.currentTimeMillis() - initializationStartTime); + telemetrySynchronizer.flush(); + telemetrySynchronizer.destroy(); + Logger.d("Successful shutdown of telemetry"); + impressionsLoggingTaskExecutor.shutdown(); + impressionsObserverExecutor.shutdown(); + Logger.d("Successful shutdown of impressions logging executor"); + mSyncManager.stop(); + Logger.d("Flushing impressions and events"); + mLifecycleManager.destroy(); + Logger.d("Successful shutdown of lifecycle manager"); + mFactoryMonitor.remove(mApiKey); + Logger.d("Successful shutdown of segment fetchers"); + customerImpressionListener.close(); + Logger.d("Successful shutdown of ImpressionListener"); + defaultHttpClient.close(); + Logger.d("Successful shutdown of httpclient"); + mManager.destroy(); + Logger.d("Successful shutdown of manager"); + splitTaskExecutor.stop(); + splitSingleThreadTaskExecutor.stop(); + Logger.d("Successful shutdown of task executor"); + mStorageContainer.getAttributesStorageContainer().destroy(); + Logger.d("Successful shutdown of attributes storage"); + mIsTerminated = true; + Logger.d("SplitFactory has been destroyed"); + } else { + Logger.d("Avoiding shutdown of factory due to active clients"); + } } catch (Exception e) { Logger.e(e, "We could not shutdown split"); } finally { - mIsTerminated = true; - Logger.d("SplitFactory has been destroyed"); mInitLock.unlock(); } } @@ -388,7 +395,22 @@ public SplitManager manager() { public void destroy() { synchronized (SplitFactoryImpl.class) { if (!mIsTerminated) { - new Thread(mDestroyer).start(); + ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor(); + executor.schedule(mDestroyer, 100, TimeUnit.MILLISECONDS); + executor.schedule(new Runnable() { + @Override + public void run() { + executor.shutdown(); + try { + if (!executor.awaitTermination(5, TimeUnit.SECONDS)) { + executor.shutdownNow(); + } + } catch (InterruptedException e) { + executor.shutdownNow(); + Thread.currentThread().interrupt(); + } + } + }, 500, TimeUnit.MILLISECONDS); } } } From 111c6db8719860584c89387815ca611e9e53f5d8 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 6 Feb 2025 15:47:39 -0300 Subject: [PATCH 3/5] Version 5.1.1-rc1 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 37af751b0..2e5689ce4 100644 --- a/build.gradle +++ b/build.gradle @@ -17,7 +17,7 @@ apply plugin: 'kotlin-android' apply from: 'spec.gradle' ext { - splitVersion = '5.1.0' + splitVersion = '5.1.1-rc1' } android { From 0c6d95269200e3ffa0ab02ad464958b914d757c9 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Mon, 10 Feb 2025 12:45:08 -0300 Subject: [PATCH 4/5] Version 5.1.1-rc2 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 2e5689ce4..6599eafce 100644 --- a/build.gradle +++ b/build.gradle @@ -17,7 +17,7 @@ apply plugin: 'kotlin-android' apply from: 'spec.gradle' ext { - splitVersion = '5.1.1-rc1' + splitVersion = '5.1.1-rc2' } android { From 2d3885497599b70a925dca62f4b551937ffca2c0 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 11 Feb 2025 10:45:25 -0300 Subject: [PATCH 5/5] Prepare release 5.1.1 --- CHANGES.txt | 3 +++ build.gradle | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index 5df09b929..90d2171e6 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,6 @@ +5.1.1 (Feb 11, 2025) +- Fixed issue when calling destroy() before the SDK has initialized. + 5.1.0 (Jan 17, 2025) - Added support for the new impressions tracking toggle available on feature flags, both respecting the setting and including the new field being returned on SplitView type objects. Read more in our docs. diff --git a/build.gradle b/build.gradle index 6599eafce..7eb29e1da 100644 --- a/build.gradle +++ b/build.gradle @@ -17,7 +17,7 @@ apply plugin: 'kotlin-android' apply from: 'spec.gradle' ext { - splitVersion = '5.1.1-rc2' + splitVersion = '5.1.1' } android {