[maps-sdk] Fix correctness issues insafeSystemCallWithCallback (#15384)

### Summary
- Removed `withTimeoutOrNull` timeout from the helper:
`display?.refreshRate` is a plain blocking call with no suspension
points, so coroutine cancellation is never checked while the IPC is in
flight; additionally, `runInterruptible` would not help either as the
native Binder layer ignores `Thread.interrupt()`
- Fixed `CancellationException` being silently swallowed by `catch (e:
Exception)`, which suppressed scope cancellation and logged a spurious
error
- Switched default dispatcher from `Dispatchers.Default` to
`Dispatchers.IO`, which has an elastic thread pool suited for blocking
I/O rather than a fixed CPU-bound pool
- Replaced `safeSystemCallWithCallback` with a simpler `safeSystemCall`
that returns `T` directly — the callback and `mainDispatcher` parameter
were unnecessary since after `withContext(Dispatchers.IO)` returns the
coroutine is already back on the caller's dispatcher

### Context
The `safeSystemCallWithCallback` helper was introduced to move
`display?.refreshRate` (a blocking Binder IPC) off the main thread. This
PR addresses correctness issues in that helper discovered during review
of MAPSAND-2266.

### Key Changes
- **`Utils.kt`**: Removed `safeSystemCallWithCallback`; replaced with
`safeSystemCall` returning `T` directly, with
`withContext(Dispatchers.IO)` and explicit rethrow of
`CancellationException`
- **`MapView.kt`, `MapSurface.kt`**: Updated call sites to use
`safeSystemCall`
- **`SafeSystemCallTest`**: Removed timeout tests; added
`CancellationException` rethrow coverage

### Validation
- [x] ktlint and lint pass
- [x] `SafeSystemCallTest`, `MapViewTest`, `MapSurfaceTest` pass

cc @mapbox/maps-android

---------

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
GitOrigin-RevId: 61119c3e820e31b9eb5b34908f0c9390cb9b8a2b

Author: 2 people

maps-sdk/src/main/java/com/mapbox/maps/MapSurface.kt

@@ -101,21 +101,16 @@ class MapSurface : MapPluginProviderDelegate, MapControllable {
     mapController.setScreenRefreshRate(MapView.DEFAULT_FPS)
     // Retrieve screen refresh rate off the main thread to prevent ANR
     mapController.lifecycleScope.launch {
-      safeSystemCallWithCallback(
-        fallback = MapView.DEFAULT_FPS,
-        logTag = TAG,
-        operation = {
-          if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
-            context.display.refreshRate.toInt()
-          } else {
-            @Suppress("DEPRECATION")
-            (context.getSystemService(Context.WINDOW_SERVICE) as WindowManager?)
-              ?.defaultDisplay?.refreshRate?.toInt() ?: MapView.DEFAULT_FPS
-          }
+      val rate = safeBinderCall(fallback = MapView.DEFAULT_FPS, logTag = TAG) {
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
+          context.display.refreshRate.toInt()
+        } else {
+          @Suppress("DEPRECATION")
+          (context.getSystemService(Context.WINDOW_SERVICE) as WindowManager?)
+            ?.defaultDisplay?.refreshRate?.toInt() ?: MapView.DEFAULT_FPS
         }
-      ) { screenRefreshRate ->
-        mapController.setScreenRefreshRate(screenRefreshRate)
       }
+      mapController.setScreenRefreshRate(rate)
     }
 
     // Stop any previous display refresh rate monitor in case `surfaceCreated` is called twice

maps-sdk/src/main/java/com/mapbox/maps/MapView.kt

@@ -255,15 +255,10 @@ open class MapView : FrameLayout, MapPluginProviderDelegate, MapControllable {
     mapController.setScreenRefreshRate(DEFAULT_FPS)
     // Retrieve screen refresh rate off the main thread to prevent ANR
     mapController.lifecycleScope.launch {
-      safeSystemCallWithCallback(
-        fallback = DEFAULT_FPS,
-        logTag = TAG,
-        operation = {
-          display?.refreshRate?.toInt() ?: DEFAULT_FPS
-        }
-      ) { screenRefreshRate ->
-        mapController.setScreenRefreshRate(screenRefreshRate)
+      val rate = safeBinderCall(fallback = DEFAULT_FPS, logTag = TAG) {
+        display?.refreshRate?.toInt() ?: DEFAULT_FPS
       }
+      mapController.setScreenRefreshRate(rate)
     }
 
     // Stop any previous display refresh rate monitor in case `onStart` is called twice

maps-sdk/src/main/java/com/mapbox/maps/Utils.kt

@@ -4,11 +4,12 @@ import android.content.Context
 import android.util.TypedValue
 import com.mapbox.bindgen.Expected
 import com.mapbox.common.Cancelable
+import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.CoroutineDispatcher
+import kotlinx.coroutines.CoroutineName
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.suspendCancellableCoroutine
 import kotlinx.coroutines.withContext
-import kotlinx.coroutines.withTimeoutOrNull
 import java.lang.ref.WeakReference
 import kotlin.coroutines.Continuation
 
@@ -40,63 +41,30 @@ internal fun <T : Number> T.toDP(context: Context): T {
 }
 
 /**
- * Executes a potentially blocking system call off the main thread with timeout and error handling.
- * This utility helps prevent ANRs caused by blocking IPC calls to system services.
+ * Executes a potentially blocking system call off the main thread, preventing ANRs.
  *
- * Uses [Dispatchers.Default] as system calls are CPU-bound operations involving IPC to system services.
+ * The call runs on [dispatcher] (default [Dispatchers.IO]). Note: if the underlying
+ * operation is a blocking Binder IPC (e.g. [android.view.Display.getRefreshRate]), its
+ * duration cannot be reliably bounded — the native Binder layer ignores [Thread.interrupt],
+ * so the dispatcher thread may stay blocked until the system server replies. The main thread
+ * is never affected.
  *
- * @param timeoutMs Timeout in milliseconds for the operation (default: 5000ms)
- * @param fallback Fallback value to return if the operation fails or times out
- * @param logTag Tag for logging errors (default: "SystemCall")
- * @param dispatcher The dispatcher to use for the operation (default: Dispatchers.Default)
- * @param operation The blocking operation to execute
- * @return The result of the operation or the fallback value
+ * @param fallback Value returned if the operation throws.
+ * @param logTag Tag for error logging.
+ * @param dispatcher Dispatcher for the blocking operation (default: [Dispatchers.IO]).
+ * @param operation The blocking operation to execute.
+ * @return The result of [operation], or [fallback] if it throws.
  */
-internal suspend fun <T> safeSystemCall(
-  timeoutMs: Long = 5000L,
+internal suspend fun <T> safeBinderCall(
   fallback: T,
   logTag: String = "SystemCall",
-  dispatcher: CoroutineDispatcher = Dispatchers.Default,
-  operation: suspend () -> T
-): T {
-  return try {
-    withTimeoutOrNull(timeoutMs) {
-      withContext(dispatcher) {
-        operation()
-      }
-    } ?: run {
-      logW(logTag, "System call timed out after ${timeoutMs}ms, using fallback")
-      fallback
-    }
-  } catch (e: Exception) {
-    logE(logTag, "System call failed: ${e.message}, using fallback")
-    fallback
-  }
-}
-
-/**
- * Executes a potentially blocking system call off the main thread and posts the result back to the main thread.
- * This is useful for updating UI components after retrieving system information.
- *
- * @param timeoutMs Timeout in milliseconds for the operation (default: 5000ms)
- * @param fallback Fallback value to return if the operation fails or times out
- * @param logTag Tag for logging errors (default: "SystemCall")
- * @param dispatcher The dispatcher to use for the operation (default: Dispatchers.Default)
- * @param mainDispatcher The dispatcher to use for the callback (default: Dispatchers.Main)
- * @param operation The blocking operation to execute
- * @param onResult Callback to handle the result on the main thread
- */
-internal suspend fun <T> safeSystemCallWithCallback(
-  timeoutMs: Long = 5000L,
-  fallback: T,
-  logTag: String = "SystemCall",
-  dispatcher: CoroutineDispatcher = Dispatchers.Default,
-  mainDispatcher: CoroutineDispatcher = Dispatchers.Main,
+  dispatcher: CoroutineDispatcher = Dispatchers.IO,
   operation: suspend () -> T,
-  onResult: (T) -> Unit
-) {
-  val result = safeSystemCall(timeoutMs, fallback, logTag, dispatcher, operation)
-  withContext(mainDispatcher) {
-    onResult(result)
-  }
+): T = try {
+  withContext(dispatcher + CoroutineName("safeBinderCall")) { operation() }
+} catch (e: CancellationException) {
+  throw e
+} catch (e: Exception) {
+  logE(logTag, "System call failed: ${e.message}, using fallback")
+  fallback
 }

maps-sdk/src/test/java/com/mapbox/maps/SafeBinderCallTest.kt

@@ -0,0 +1,81 @@
+package com.mapbox.maps
+
+import com.mapbox.verifyOnce
+import io.mockk.Runs
+import io.mockk.every
+import io.mockk.just
+import io.mockk.mockkStatic
+import io.mockk.unmockkStatic
+import kotlinx.coroutines.CancellationException
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.TestCoroutineScheduler
+import kotlinx.coroutines.test.UnconfinedTestDispatcher
+import kotlinx.coroutines.test.resetMain
+import kotlinx.coroutines.test.runTest
+import kotlinx.coroutines.test.setMain
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertThrows
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+
+@OptIn(ExperimentalCoroutinesApi::class)
+@RunWith(RobolectricTestRunner::class)
+class SafeBinderCallTest {
+
+  private val testScheduler = TestCoroutineScheduler()
+  private val testDispatcher = UnconfinedTestDispatcher(testScheduler)
+
+  @Before
+  fun setUp() {
+    Dispatchers.setMain(testDispatcher)
+    mockkStatic("com.mapbox.maps.MapboxLogger")
+    every { logI(any(), any()) } just Runs
+    every { logW(any(), any()) } just Runs
+    every { logE(any(), any()) } just Runs
+  }
+
+  @After
+  fun cleanUp() {
+    Dispatchers.resetMain()
+    unmockkStatic("com.mapbox.maps.MapboxLogger")
+  }
+
+  @Test
+  fun `safeBinderCall should return operation result when successful`() = runTest {
+    val result = safeBinderCall(fallback = 0, logTag = "Test", dispatcher = testDispatcher) { 42 }
+    assertEquals(42, result)
+  }
+
+  @Test
+  fun `safeBinderCall should return fallback when operation throws`() = runTest {
+    val result = safeBinderCall(fallback = 100, logTag = "Test", dispatcher = testDispatcher) {
+      throw RuntimeException("Test exception")
+    }
+    assertEquals(100, result)
+    verifyOnce { logE("Test", "System call failed: Test exception, using fallback") }
+  }
+
+  @Test
+  fun `safeBinderCall should rethrow CancellationException`() = runTest {
+    var reached = false
+    assertThrows(CancellationException::class.java) {
+      kotlinx.coroutines.runBlocking {
+        safeBinderCall(fallback = 0, logTag = "Test", dispatcher = testDispatcher) {
+          throw CancellationException("scope cancelled")
+        }
+        reached = true
+      }
+    }
+    assertEquals(false, reached)
+  }
+
+  @Test
+  fun `safeBinderCall should handle null fallback`() = runTest {
+    val result = safeBinderCall<String?>(fallback = null, logTag = "Test", dispatcher = testDispatcher) { null }
+    assertEquals(null, result)
+  }
+}