Skip to content

Commit

Permalink
Avoid creating threads when dispatching tasks (#1496)
Browse files Browse the repository at this point in the history
### Description
We were creating threads every time we added a task to the executor
service. This shouldn't be a problem if tasks ended early and not too
many tasks were in the queue, but it's not very efficient. This is
another implementation to propagate exceptions to the main thread so
they can be thrown and not swallowed when they happen on the thread
pool.
  • Loading branch information
tonidero committed Nov 24, 2023
1 parent 2967dfc commit b4cd7d2
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,15 @@ internal class PurchasesFactory(

val eTagManager = ETagManager(context)

val dispatcher = Dispatcher(createDefaultExecutor(), runningIntegrationTests)
val backendDispatcher = Dispatcher(service ?: createDefaultExecutor(), runningIntegrationTests)
val eventsDispatcher = Dispatcher(createEventsExecutor(), runningIntegrationTests)
val dispatcher = Dispatcher(createDefaultExecutor(), runningIntegrationTests = runningIntegrationTests)
val backendDispatcher = Dispatcher(
service ?: createDefaultExecutor(),
runningIntegrationTests = runningIntegrationTests,
)
val eventsDispatcher = Dispatcher(
createEventsExecutor(),
runningIntegrationTests = runningIntegrationTests,
)

var diagnosticsFileHelper: DiagnosticsFileHelper? = null
var diagnosticsTracker: DiagnosticsTracker? = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package com.revenuecat.purchases.common

import android.os.Handler
import android.os.Looper
import com.revenuecat.purchases.PurchasesError
import com.revenuecat.purchases.common.networking.HTTPResult
import com.revenuecat.purchases.common.verification.SignatureVerificationException
Expand All @@ -24,6 +26,7 @@ internal enum class Delay(val minDelay: Duration, val maxDelay: Duration) {

internal open class Dispatcher(
private val executorService: ExecutorService,
private val mainHandler: Handler? = Handler(Looper.getMainLooper()),
private val runningIntegrationTests: Boolean = false,
) {
private companion object {
Expand Down Expand Up @@ -59,28 +62,26 @@ internal open class Dispatcher(
) {
synchronized(this.executorService) {
if (!executorService.isShutdown) {
val future = if (delay != Delay.NONE && executorService is ScheduledExecutorService) {
val commandHandlingExceptions = Runnable {
try {
command.run()
} catch (@Suppress("TooGenericExceptionCaught") e: Exception) {
errorLog("Exception running command: $e")
// We propagate the exception to the main thread to be sure it's not swallowed.
mainHandler?.post {
throw e
}
}
}
if (delay != Delay.NONE && executorService is ScheduledExecutorService) {
var delayToApply = (delay.minDelay.inWholeMilliseconds..delay.maxDelay.inWholeMilliseconds).random()
if (runningIntegrationTests) {
delayToApply = (delayToApply * INTEGRATION_TEST_DELAY_PERCENTAGE).toLong()
}
executorService.schedule(command, delayToApply, TimeUnit.MILLISECONDS)
executorService.schedule(commandHandlingExceptions, delayToApply, TimeUnit.MILLISECONDS)
} else {
executorService.submit(command)
executorService.submit(commandHandlingExceptions)
}

// Exceptions are being swallowed if using execute instead of submit
// Future.get is blocking so we create a Thread
// More info: https://github.com/RevenueCat/purchases-android/pull/234
Thread {
try {
future.get()
} catch (e: InterruptedException) {
Thread.currentThread().interrupt()
} catch (@Suppress("TooGenericExceptionCaught") e: Exception) {
e.cause?.let { throw it }
}
}.start()
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import com.revenuecat.purchases.common.Dispatcher
import io.mockk.mockk
import java.util.concurrent.RejectedExecutionException

internal class SyncDispatcher : Dispatcher(mockk()) {
internal class SyncDispatcher : Dispatcher(mockk(), MockHandlerFactory.createMockHandler()) {

private var closed = false

Expand Down

0 comments on commit b4cd7d2

Please sign in to comment.