[sdk] add thread checking (#1316)

* [sdk] Enable main thread checking on the map object when running applications in debug mode. * update changelog
mapbox · Jun 29, 2022 · 64e3c47 · 64e3c47
1 parent a35ee8c
commit 64e3c47
Show file tree

Hide file tree

Showing 21 changed files with 1,310 additions and 714 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@ Mapbox welcomes participation and contributions from everyone.
 * Remove `MapboxExperimental` annotation from viewport plugin and promote viewport plugin as stable API. ([1425](https://github.com/mapbox/mapbox-maps-android/pull/1425))
 * Introduce `addRendererSetupErrorListener`/`removeRendererSetupErrorListener` methods for `MapView` and `MapSurface` to listen to renderer setup errors and give opportunity to control some edge cases. ([1427](https://github.com/mapbox/mapbox-maps-android/pull/1427))
 * Introduce transition properties for atmosphere and terrain. ([1451](https://github.com/mapbox/mapbox-maps-android/pull/1451))
+* Enable main thread checking on the map/style object when running applications in debug build. This utility class will crash the application if these objects are accessed from a worked thread. It's required to call these object functions on the main thread, otherwise you can hit edge case crashes. This configurations is advised but can be opted out with a Manifest metadata entry of `com.mapbox.maps.ThreadChecker` and corresponding false value. ([#1316](https://github.com/mapbox/mapbox-maps-android/pull/1316)). 
 * Introduce view annotation `ViewAnnotationManager.setViewAnnotationUpdateMode` / `ViewAnnotationManager.getViewAnnotationUpdateMode` API with following synchronization modes: MAP_SYNCHRONIZED (used by default) and MAP_FIXED_DELAY. ([1415](https://github.com/mapbox/mapbox-maps-android/pull/1415))
 * Reduce geometry on globe tile to increase rendering performance. ([#1462](https://github.com/mapbox/mapbox-maps-android/pull/1462))
 * Improve rendering performance with deleting layer render data on a worker thread. ([#1462](https://github.com/mapbox/mapbox-maps-android/pull/1462))

diff --git a/app/src/androidTest/java/com/mapbox/maps/testapp/annotation/UpdateAnnotationTest.kt b/app/src/androidTest/java/com/mapbox/maps/testapp/annotation/UpdateAnnotationTest.kt
@@ -2,6 +2,7 @@ package com.mapbox.maps.testapp.annotation
 
 import android.graphics.Color
 import android.os.Handler
+import android.os.Looper
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.filters.LargeTest
 import com.mapbox.geojson.Point
@@ -64,43 +65,51 @@ class UpdateAnnotationTest : BaseMapTest() {
 
   override fun loadMap() {
     super.loadMap()
-    pointAnnotationManager = mapView.annotations.createPointAnnotationManager()
-    pointAnnotationManager.textFont = listOf("Open Sans Regular")
+    val latch = CountDownLatch(1)
+    handler = Handler(Looper.getMainLooper())
+    handler.post {
+      pointAnnotationManager = mapView.annotations.createPointAnnotationManager()
+      pointAnnotationManager.textFont = listOf("Open Sans Regular")
 
-    pointAnnotation = pointAnnotationManager.create(
-      PointAnnotationOptions()
-        .withIconColor(ColorUtils.colorToRgbaString(Color.RED))
-        .withIconImage("car-15")
-        .withDraggable(true)
-        .withIconAnchor(IconAnchor.CENTER)
-        .withIconHaloBlur(1.0)
-        .withIconHaloColor(ColorUtils.colorToRgbaString(Color.YELLOW))
-        .withIconHaloWidth(2.0)
-        .withIconOffset(listOf(1.0, 2.0))
-        .withIconOpacity(0.8)
-        .withIconRotate(0.5)
-        .withIconSize(5.0)
-        .withIconHaloColor(ColorUtils.colorToRgbaString(Color.WHITE))
-        .withSymbolSortKey(1.0)
-        .withTextAnchor(TextAnchor.TOP)
-        .withTextColor(ColorUtils.colorToRgbaString(Color.YELLOW))
-        .withTextField("Car")
-        .withTextHaloBlur(1.0)
-        .withTextHaloWidth(5.0)
-        .withTextJustify(TextJustify.CENTER)
-        .withTextLetterSpacing(2.0)
-        .withTextRotate(5.0)
-        .withTextTransform(TextTransform.UPPERCASE)
-        .withTextSize(15.0)
-        .withTextRadialOffset(1.0)
-        .withTextOffset(listOf(1.0, 2.0))
-        .withTextMaxWidth(10.0)
-        .withPoint(Point.fromLngLat(0.0, 0.0))
-    )
-    for (i in 0..100) {
-      // Verify there is no ConcurrentModificationException https://github.com/mapbox/mapbox-maps-android/issues/383
-      pointAnnotation.textOpacity = 0.8
-      Thread.sleep(0, 2000)
+      pointAnnotation = pointAnnotationManager.create(
+        PointAnnotationOptions()
+          .withIconColor(ColorUtils.colorToRgbaString(Color.RED))
+          .withIconImage("car-15")
+          .withDraggable(true)
+          .withIconAnchor(IconAnchor.CENTER)
+          .withIconHaloBlur(1.0)
+          .withIconHaloColor(ColorUtils.colorToRgbaString(Color.YELLOW))
+          .withIconHaloWidth(2.0)
+          .withIconOffset(listOf(1.0, 2.0))
+          .withIconOpacity(0.8)
+          .withIconRotate(0.5)
+          .withIconSize(5.0)
+          .withIconHaloColor(ColorUtils.colorToRgbaString(Color.WHITE))
+          .withSymbolSortKey(1.0)
+          .withTextAnchor(TextAnchor.TOP)
+          .withTextColor(ColorUtils.colorToRgbaString(Color.YELLOW))
+          .withTextField("Car")
+          .withTextHaloBlur(1.0)
+          .withTextHaloWidth(5.0)
+          .withTextJustify(TextJustify.CENTER)
+          .withTextLetterSpacing(2.0)
+          .withTextRotate(5.0)
+          .withTextTransform(TextTransform.UPPERCASE)
+          .withTextSize(15.0)
+          .withTextRadialOffset(1.0)
+          .withTextOffset(listOf(1.0, 2.0))
+          .withTextMaxWidth(10.0)
+          .withPoint(Point.fromLngLat(0.0, 0.0))
+      )
+      for (i in 0..100) {
+        // Verify there is no ConcurrentModificationException https://github.com/mapbox/mapbox-maps-android/issues/383
+        pointAnnotation.textOpacity = 0.8
+        Thread.sleep(0, 2000)
+      }
+      latch.countDown()
+    }
+    if (!latch.await(3000, TimeUnit.MILLISECONDS)) {
+      throw TimeoutException()
     }
     Assert.assertEquals(pointAnnotation, pointAnnotationManager.annotations[0])
   }

diff --git a/...roidTest/java/com/mapbox/maps/testapp/annotation/UpdateAnnotationWithMultiManagersTest.kt b/...roidTest/java/com/mapbox/maps/testapp/annotation/UpdateAnnotationWithMultiManagersTest.kt
@@ -91,48 +91,56 @@ class UpdateAnnotationWithMultiManagersTest : BaseMapTest() {
 
           Thread {
             for (i in 1..updateTimes) {
-              pointAnnotation.point = Point.fromLngLat(
-                pointAnnotation.point.longitude() + updateSteps,
-                pointAnnotation.point.latitude() + updateSteps
-              )
-              pointAnnotationManager.update(pointAnnotation)
+              handler.post {
+                pointAnnotation.point = Point.fromLngLat(
+                  pointAnnotation.point.longitude() + updateSteps,
+                  pointAnnotation.point.latitude() + updateSteps
+                )
+                pointAnnotationManager.update(pointAnnotation)
+              }
               Thread.sleep(10)
             }
             latch.countDown()
           }.start()
 
           Thread {
             for (i in 1..updateTimes) {
-              circleAnnotation.point = Point.fromLngLat(
-                circleAnnotation.point.longitude() + updateSteps,
-                circleAnnotation.point.latitude() + updateSteps
-              )
-              circleAnnotationManager.update(circleAnnotation)
+              handler.post {
+                circleAnnotation.point = Point.fromLngLat(
+                  circleAnnotation.point.longitude() + updateSteps,
+                  circleAnnotation.point.latitude() + updateSteps
+                )
+                circleAnnotationManager.update(circleAnnotation)
+              }
               Thread.sleep(10)
             }
             latch.countDown()
           }.start()
 
           Thread {
             for (i in 1..updateTimes) {
-              points[0][0] = Point.fromLngLat(
-                points[0][0].longitude() + updateSteps,
-                points[0][0].latitude() + updateSteps
-              )
-              polygonAnnotation.points = points
-              polygonAnnotationManager.update(polygonAnnotation)
+              handler.post {
+                points[0][0] = Point.fromLngLat(
+                  points[0][0].longitude() + updateSteps,
+                  points[0][0].latitude() + updateSteps
+                )
+                polygonAnnotation.points = points
+                polygonAnnotationManager.update(polygonAnnotation)
+              }
               Thread.sleep(10)
             }
             latch.countDown()
           }.start()
           Thread {
             for (i in 1..updateTimes) {
-              polylinePoints[0] = Point.fromLngLat(
-                polylinePoints[0].longitude() + updateSteps,
-                polylinePoints[0].latitude() + updateSteps
-              )
-              polylineAnnotation.points = polylinePoints
-              polylineAnnotationManager.update(polylineAnnotation)
+              handler.post {
+                polylinePoints[0] = Point.fromLngLat(
+                  polylinePoints[0].longitude() + updateSteps,
+                  polylinePoints[0].latitude() + updateSteps
+                )
+                polylineAnnotation.points = polylinePoints
+                polylineAnnotationManager.update(polylineAnnotation)
+              }
               Thread.sleep(10)
             }
             latch.countDown()