Safe util: additional tests and timeout adjustments

fingerprintjs · Sep 29, 2023 · 6c2e32d · 6c2e32d
1 parent 1d3d7d7
commit 6c2e32d
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 9 deletions.
diff --git a/fingerprint/src/main/java/com/fingerprintjs/android/fingerprint/Fingerprinter.kt b/fingerprint/src/main/java/com/fingerprintjs/android/fingerprint/Fingerprinter.kt
@@ -19,6 +19,7 @@ import com.fingerprintjs.android.fingerprint.tools.hashers.Hasher
 import com.fingerprintjs.android.fingerprint.tools.hashers.MurMur3x64x128Hasher
 import com.fingerprintjs.android.fingerprint.tools.logs.Logger
 import com.fingerprintjs.android.fingerprint.tools.logs.ePleaseReport
+import com.fingerprintjs.android.fingerprint.tools.safe.Safe
 import com.fingerprintjs.android.fingerprint.tools.safe.safe
 import com.fingerprintjs.android.fingerprint.tools.safe.safeAsync
 
@@ -242,7 +243,7 @@ public class Fingerprinter internal constructor(
         fingerprintingSignals: List<FingerprintingSignal<*>>,
         hasher: Hasher = MurMur3x64x128Hasher(),
     ): String {
-        return safe { hasher.hash(fingerprintingSignals) }
+        return safe(timeoutMs = Safe.timeoutLong) { hasher.hash(fingerprintingSignals) }
             .onFailure { Logger.ePleaseReport(it) }
             .getOrDefault(DummyResults.fingerprint)
     }

diff --git a/fingerprint/src/main/java/com/fingerprintjs/android/fingerprint/tools/safe/Safe.kt b/fingerprint/src/main/java/com/fingerprintjs/android/fingerprint/tools/safe/Safe.kt
@@ -1,26 +1,42 @@
 package com.fingerprintjs.android.fingerprint.tools.safe
 
+import androidx.annotation.VisibleForTesting
 import java.util.concurrent.Callable
-import java.util.concurrent.Executors
+import java.util.concurrent.SynchronousQueue
+import java.util.concurrent.ThreadPoolExecutor
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.TimeoutException
 import java.util.concurrent.atomic.AtomicReference
 
-private const val DEFAULT_CALL_AWAIT_INTERVAL_MS = 3_000L
-private val executor = Executors.newCachedThreadPool()
+internal object Safe {
+    const val timeoutShort = 1_000L
+    const val timeoutLong = 3_000L
+
+    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+    internal var executor = createThreadPoolExecutor()
+
+    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+    internal fun createThreadPoolExecutor(): ThreadPoolExecutor =
+        // defaults from Executors.newCachedThreadPool()
+        ThreadPoolExecutor(
+            0, Integer.MAX_VALUE,
+            60L, TimeUnit.SECONDS,
+            SynchronousQueue()
+        )
+}
 
 /**
  * Runs the [block], catching all exceptions and handling unexpected execution locks (configured with [timeoutMs]).
  */
 internal fun <T> safe(
-    timeoutMs: Long = DEFAULT_CALL_AWAIT_INTERVAL_MS,
+    timeoutMs: Long = Safe.timeoutShort,
     block: () -> T,
 ): Result<T> {
     // we can't make a local variable volatile, hence using atomic reference here
     val executionThread = AtomicReference<Thread>(null)
 
     val future = runCatching {
-        executor.submit(
+        Safe.executor.submit(
             Callable {
                 executionThread.set(Thread.currentThread())
                 block()
@@ -45,12 +61,14 @@ internal fun <T> safe(
 }
 
 internal fun safeAsync(
-    timeoutMs: Long = DEFAULT_CALL_AWAIT_INTERVAL_MS,
+    // using long timeout here because this function is used at the highest level and
+    // may contain multiple safe calls inside
+    timeoutMs: Long = Safe.timeoutLong,
     onError: (Throwable) -> Unit = {},
     block: () -> Unit,
 ) {
     runCatching {
-        executor.execute {
+        Safe.executor.execute {
             safe(timeoutMs = timeoutMs) { block() }
                 .onFailure(onError)
         }
@@ -59,7 +77,7 @@ internal fun safeAsync(
 }
 
 internal fun <T> safeLazy(
-    timeoutMs: Long = DEFAULT_CALL_AWAIT_INTERVAL_MS,
+    timeoutMs: Long = Safe.timeoutShort,
     constructor: () -> T,
 ): SafeLazy<T> = SafeLazy(timeoutMs, constructor)
 

diff --git a/fingerprint/src/test/java/com/fingerprintjs/android/fingerprint/SafeTests.kt b/fingerprint/src/test/java/com/fingerprintjs/android/fingerprint/SafeTests.kt
@@ -1,10 +1,12 @@
 package com.fingerprintjs.android.fingerprint
 
 import com.fingerprintjs.android.fingerprint.tools.safe.ExecutionTimeoutException
+import com.fingerprintjs.android.fingerprint.tools.safe.Safe
 import com.fingerprintjs.android.fingerprint.tools.safe.safe
 import com.fingerprintjs.android.fingerprint.tools.safe.safeAsync
 import com.fingerprintjs.android.fingerprint.tools.safe.safeLazy
 import junit.framework.TestCase
+import org.junit.After
 import org.junit.Test
 import java.util.concurrent.CountDownLatch
 import java.util.concurrent.ExecutionException
@@ -13,6 +15,11 @@ import java.util.concurrent.atomic.AtomicInteger
 
 class SafeTests {
 
+    @After
+    fun recreateExecutor() {
+        Safe.executor = Safe.createThreadPoolExecutor()
+    }
+
     @Test
     fun safeValueReturned() {
         val v = safe { 0 }
@@ -64,6 +71,59 @@ class SafeTests {
         TestCase.assertTrue(elapsedTime - TimeConstants.t1 < TimeConstants.epsilon)
     }
 
+    @Test
+    fun safeThreadsAreReused() {
+        for (i in 0 until 4) {
+            safe { }
+            TestCase.assertEquals(1, Safe.executor.poolSize)
+            Thread.sleep(TimeConstants.epsilon)
+        }
+    }
+
+    // this is a sad fact but we will leave it as it is
+    @Test
+    fun safeThreadCountGrowsIfThreadsCantInterrupt() {
+        for (i in 1 until 5) {
+            safe(timeoutMs = TimeConstants.epsilon) { neverReturn() }
+            TestCase.assertEquals(i, Safe.executor.poolSize)
+            Thread.sleep(TimeConstants.epsilon)
+        }
+    }
+
+    @Test
+    fun safeOuterTimeoutDominatesOverInner() {
+        val elapsedTime = elapsedTimeMs {
+            safe(timeoutMs = TimeConstants.t1) {
+                safe(timeoutMs = TimeConstants.t2) {
+                    Thread.sleep(TimeConstants.t3)
+                }
+            }
+        }
+        TestCase.assertTrue(elapsedTime - TimeConstants.t1 < TimeConstants.epsilon)
+    }
+
+    @Test
+    fun safeNestedSafeInterrupted() {
+        val errLvl1: Throwable?
+        var errLvl2: Throwable? = null
+        var errLvl3: Throwable? = null
+        val countDownLatch = CountDownLatch(1)
+        errLvl1 = safe(timeoutMs = TimeConstants.t1) {
+            errLvl2 = safe(timeoutMs = TimeConstants.t2) {
+                try {
+                    Thread.sleep(TimeConstants.t3)
+                } catch (t: Throwable) {
+                    errLvl3 = t
+                    countDownLatch.countDown()
+                }
+            }.exceptionOrNull()
+        }.exceptionOrNull()
+        countDownLatch.await()
+        TestCase.assertTrue(errLvl1 is ExecutionTimeoutException)
+        TestCase.assertTrue(errLvl2 is InterruptedException)
+        TestCase.assertTrue(errLvl3 is InterruptedException)
+    }
+
     @Suppress("VARIABLE_WITH_REDUNDANT_INITIALIZER")
     @Test
     fun safeLazyEvaluatedOnce() {
@@ -111,6 +171,14 @@ class SafeTests {
         TestCase.assertEquals(1, atomicInteger.get())
     }
 
+    @Test
+    fun safeLazyNestedInterrupted() {
+        val v1 = safeLazy(timeoutMs = TimeConstants.t1) { Thread.sleep(TimeConstants.t2) }
+        val v2 = safe(timeoutMs = TimeConstants.epsilon) { v1.getOrThrow() }
+        TestCase.assertTrue(v1.res.exceptionOrNull() is InterruptedException)
+        TestCase.assertTrue(v2.exceptionOrNull() is ExecutionTimeoutException)
+    }
+
     @Test
     fun safeAsyncValueReturned() {
         val countDownLatch = CountDownLatch(1)
@@ -195,3 +263,8 @@ private inline fun elapsedTimeMs(block: () -> Unit): Long {
     block()
     return System.currentTimeMillis() - currentTime
 }
+
+@Suppress("ControlFlowWithEmptyBody")
+private fun neverReturn() {
+    while (true);
+}