Merge pull request #2616 from DataDog/nogorodnikov/rum-9655/fix-semantincs-of-executorservice-submit-vs-executor-execute-usage

RUM-9655: Fix semantics of `ExecutorService.submit` vs `Executor.execute` usage

Author: 0xnm

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt

@@ -71,7 +71,7 @@ import com.datadog.android.core.internal.user.DatadogUserInfoProvider
 import com.datadog.android.core.internal.user.MutableUserInfoProvider
 import com.datadog.android.core.internal.user.NoOpMutableUserInfoProvider
 import com.datadog.android.core.internal.user.UserInfoDeserializer
-import com.datadog.android.core.internal.utils.submitSafe
+import com.datadog.android.core.internal.utils.executeSafe
 import com.datadog.android.core.internal.utils.unboundInternalLogger
 import com.datadog.android.core.persistence.PersistenceStrategy
 import com.datadog.android.core.thread.FlushableExecutorService
@@ -201,7 +201,7 @@ internal class CoreFeature(
         readApplicationInformation(appContext, configuration)
         resolveProcessInfo(appContext)
         setupExecutors()
-        persistenceExecutorService.submitSafe("NTP Sync initialization", unboundInternalLogger) {
+        persistenceExecutorService.executeSafe("NTP Sync initialization", unboundInternalLogger) {
             // Kronos performs I/O operation on startup, it needs to run in background
             initializeClockSync(appContext)
         }

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/DatadogCore.kt

@@ -34,8 +34,8 @@ import com.datadog.android.core.internal.logger.SdkInternalLogger
 import com.datadog.android.core.internal.net.FirstPartyHostHeaderTypeResolver
 import com.datadog.android.core.internal.system.BuildSdkVersionProvider
 import com.datadog.android.core.internal.time.DefaultAppStartTimeProvider
+import com.datadog.android.core.internal.utils.executeSafe
 import com.datadog.android.core.internal.utils.scheduleSafe
-import com.datadog.android.core.internal.utils.submitSafe
 import com.datadog.android.core.thread.FlushableExecutorService
 import com.datadog.android.error.internal.CrashReportsFeature
 import com.datadog.android.internal.telemetry.InternalTelemetryEvent
@@ -182,7 +182,7 @@ internal class DatadogCore(
         features.values.forEach {
             it.clearAllData()
         }
-        getPersistenceExecutorService().submitSafe("Clear all data", internalLogger) {
+        getPersistenceExecutorService().executeSafe("Clear all data", internalLogger) {
             coreFeature.deleteLastViewEvent()
             coreFeature.deleteLastFatalAnrSent()
         }

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/AbstractStorage.kt

@@ -16,7 +16,7 @@ import com.datadog.android.api.storage.FeatureStorageConfiguration
 import com.datadog.android.api.storage.RawBatchEvent
 import com.datadog.android.core.internal.metrics.RemovalReason
 import com.datadog.android.core.internal.privacy.ConsentProvider
-import com.datadog.android.core.internal.utils.submitSafe
+import com.datadog.android.core.internal.utils.executeSafe
 import com.datadog.android.core.persistence.NoOpPersistenceStrategy
 import com.datadog.android.core.persistence.PersistenceStrategy
 import com.datadog.android.privacy.TrackingConsent
@@ -64,7 +64,7 @@ internal class AbstractStorage(
         forceNewBatch: Boolean,
         callback: (EventBatchWriter) -> Unit
     ) {
-        executorService.submitSafe("Data write", internalLogger) {
+        executorService.executeSafe("Data write", internalLogger) {
             val strategy = resolvePersistenceStrategy()
             val writer = object : EventBatchWriter {
                 @WorkerThread
@@ -115,7 +115,7 @@ internal class AbstractStorage(
 
     @AnyThread
     override fun dropAll() {
-        executorService.submitSafe("Data drop", internalLogger) {
+        executorService.executeSafe("Data drop", internalLogger) {
             grantedPersistenceStrategy.dropAll()
             pendingPersistenceStrategy.dropAll()
         }
@@ -129,7 +129,7 @@ internal class AbstractStorage(
         previousConsent: TrackingConsent,
         newConsent: TrackingConsent
     ) {
-        executorService.submitSafe("Data migration", internalLogger) {
+        executorService.executeSafe("Data migration", internalLogger) {
             if (previousConsent == TrackingConsent.PENDING) {
                 when (newConsent) {
                     TrackingConsent.GRANTED -> pendingPersistenceStrategy.migrateData(grantedPersistenceStrategy)

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/ConsentAwareStorage.kt

@@ -23,7 +23,7 @@ import com.datadog.android.core.internal.persistence.file.batch.BatchFileReaderW
 import com.datadog.android.core.internal.persistence.file.existsSafe
 import com.datadog.android.core.internal.persistence.file.lengthSafe
 import com.datadog.android.core.internal.privacy.ConsentProvider
-import com.datadog.android.core.internal.utils.submitSafe
+import com.datadog.android.core.internal.utils.executeSafe
 import com.datadog.android.core.metrics.MethodCallSamplingRate
 import com.datadog.android.core.metrics.TelemetryMetricType
 import com.datadog.android.privacy.TrackingConsent
@@ -66,12 +66,12 @@ internal class ConsentAwareStorage(
             samplingRate = MethodCallSamplingRate.RARE.rate,
             operationName = "writeCurrentBatch[$featureName]"
         )
-        executorService.submitSafe("Data write", internalLogger) {
+        executorService.executeSafe("Data write", internalLogger) {
             val orchestrator = resolveOrchestrator()
             if (orchestrator == null) {
                 callback.invoke(NoOpEventBatchWriter())
                 metric?.stopAndSend(false)
-                return@submitSafe
+                return@executeSafe
             }
             synchronized(writeLock) {
                 val batchFile = orchestrator.getWritableFile(forceNewBatch)
@@ -144,7 +144,7 @@ internal class ConsentAwareStorage(
     /** @inheritdoc */
     @AnyThread
     override fun dropAll() {
-        executorService.submitSafe("ConsentAwareStorage.dropAll", internalLogger) {
+        executorService.executeSafe("ConsentAwareStorage.dropAll", internalLogger) {
             synchronized(lockedBatches) {
                 lockedBatches.forEach {
                     deleteBatch(it, RemovalReason.Flushed)

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/advanced/ConsentAwareFileOrchestrator.kt

@@ -12,7 +12,7 @@ import com.datadog.android.api.InternalLogger
 import com.datadog.android.core.internal.persistence.file.FileOrchestrator
 import com.datadog.android.core.internal.persistence.file.NoOpFileOrchestrator
 import com.datadog.android.core.internal.privacy.ConsentProvider
-import com.datadog.android.core.internal.utils.submitSafe
+import com.datadog.android.core.internal.utils.executeSafe
 import com.datadog.android.privacy.TrackingConsent
 import com.datadog.android.privacy.TrackingConsentProviderCallback
 import java.io.File
@@ -98,7 +98,7 @@ internal open class ConsentAwareFileOrchestrator(
     ) {
         val previousOrchestrator = resolveDelegateOrchestrator(previousConsent)
         val newOrchestrator = resolveDelegateOrchestrator(newConsent)
-        executorService.submitSafe("Data migration", internalLogger) {
+        executorService.executeSafe("Data migration", internalLogger) {
             dataMigrator.migrateData(
                 previousConsent,
                 previousOrchestrator,

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/advanced/ScheduledWriter.kt

@@ -9,7 +9,7 @@ package com.datadog.android.core.internal.persistence.file.advanced
 import androidx.annotation.WorkerThread
 import com.datadog.android.api.InternalLogger
 import com.datadog.android.core.internal.persistence.DataWriter
-import com.datadog.android.core.internal.utils.submitSafe
+import com.datadog.android.core.internal.utils.executeSafe
 import java.util.concurrent.ExecutorService
 
 internal class ScheduledWriter<T : Any>(
@@ -22,14 +22,14 @@ internal class ScheduledWriter<T : Any>(
 
     @WorkerThread
     override fun write(element: T) {
-        executorService.submitSafe("Data writing", internalLogger) {
+        executorService.executeSafe("Data writing", internalLogger) {
             delegateWriter.write(element)
         }
     }
 
     @WorkerThread
     override fun write(data: List<T>) {
-        executorService.submitSafe("Data writing", internalLogger) {
+        executorService.executeSafe("Data writing", internalLogger) {
             delegateWriter.write(data)
         }
     }

dd-sdk-android-core/src/main/kotlin/com/datadog/android/ndk/internal/DatadogNdkCrashHandler.kt

@@ -17,7 +17,7 @@ import com.datadog.android.core.internal.persistence.file.FileReader
 import com.datadog.android.core.internal.persistence.file.existsSafe
 import com.datadog.android.core.internal.persistence.file.listFilesSafe
 import com.datadog.android.core.internal.persistence.file.readTextSafe
-import com.datadog.android.core.internal.utils.submitSafe
+import com.datadog.android.core.internal.utils.executeSafe
 import com.datadog.android.log.LogAttributes
 import com.google.gson.JsonObject
 import java.io.File
@@ -50,7 +50,7 @@ internal class DatadogNdkCrashHandler(
     // region NdkCrashHandler
 
     override fun prepareData() {
-        dataPersistenceExecutorService.submitSafe("NDK crash check", internalLogger) {
+        dataPersistenceExecutorService.executeSafe("NDK crash check", internalLogger) {
             readCrashData()
         }
     }
@@ -59,7 +59,7 @@ internal class DatadogNdkCrashHandler(
         sdkCore: FeatureSdkCore,
         reportTarget: NdkCrashHandler.ReportTarget
     ) {
-        dataPersistenceExecutorService.submitSafe("NDK crash report ", internalLogger) {
+        dataPersistenceExecutorService.executeSafe("NDK crash report ", internalLogger) {
             checkAndHandleNdkCrashReport(sdkCore, reportTarget)
         }
     }

dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/DatadogCoreInitializationTest.kt

@@ -92,10 +92,6 @@ internal class DatadogCoreInitializationTest {
         whenever(mockPersistenceExecutorService.execute(any())) doAnswer {
             it.getArgument<Runnable>(0).run()
         }
-        whenever(mockPersistenceExecutorService.submit(any())) doAnswer {
-            it.getArgument<Runnable>(0).run()
-            mock()
-        }
     }
 
     @AfterEach

dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/DatadogCoreTest.kt

@@ -79,7 +79,6 @@ import java.util.Collections
 import java.util.Locale
 import java.util.UUID
 import java.util.concurrent.CountDownLatch
-import java.util.concurrent.Future
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.concurrent.atomic.AtomicReference
@@ -122,10 +121,6 @@ internal class DatadogCoreTest {
         whenever(mockPersistenceExecutorService.execute(any())) doAnswer {
             it.getArgument<Runnable>(0).run()
         }
-        whenever(mockPersistenceExecutorService.submit(any())) doAnswer {
-            it.getArgument<Runnable>(0).run()
-            mock()
-        }
 
         testedCore = DatadogCore(
             appContext.mockInstance,
@@ -753,10 +748,9 @@ internal class DatadogCoreTest {
         testedCore.coreFeature = mockCoreFeature
         val mockExecutorService: FlushableExecutorService = mock()
         whenever(mockCoreFeature.persistenceExecutorService) doReturn mockExecutorService
-        whenever(mockExecutorService.submit(any())) doAnswer {
+        whenever(mockExecutorService.execute(any())) doAnswer {
             val runnable = it.arguments.first() as Runnable
             runnable.run()
-            mock<Future<*>>()
         }
 
         // When

dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/CoreFeatureTest.kt

@@ -155,10 +155,6 @@ internal class CoreFeatureTest {
         whenever(mockPersistenceExecutorService.execute(any())) doAnswer {
             it.getArgument<Runnable>(0).run()
         }
-        whenever(mockPersistenceExecutorService.submit(any())) doAnswer {
-            it.getArgument<Runnable>(0).run()
-            mock()
-        }
     }
 
     @AfterEach