Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 096a0bd

Browse files
authored
Fix the race condition when updating LocationCompoent's position. (#703)
* Fix the race condition when updating LocationCompoent's position. When using SymbolLayer and GeoJsonSource backed location puck, there's high frequency modifications done on the main thread and locationFeature is always re-used (modified) while being parsed on the worker thread. As the `setGeoJson` method is not thread-safe, we should do a deep copy of the location feature before passing setting it to GeoJsonSource. * Fix unit test.
1 parent 3467eef commit 096a0bd

File tree

3 files changed

+12
-2
lines changed

3 files changed

+12
-2
lines changed

MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/SymbolLocationLayerRenderer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ private void refreshSource() {
343343

344344
GeoJsonSource source = style.getSourceAs(LOCATION_SOURCE);
345345
if (source != null) {
346-
locationSource.setGeoJson(locationFeature);
346+
locationSource.setGeoJson(locationFeature.toJson());
347347
}
348348
}
349349

MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/GeoJsonSource.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,9 @@ public GeoJsonSource(String id, Geometry geometry, GeoJsonOptions options) {
256256
* Updates the GeoJson with a single feature. The update is performed asynchronously,
257257
* so the data won't be immediately visible or available to query when this method returns.
258258
*
259+
* Note: This method is not thread-safe. The Feature is parsed on a worker thread, please make sure
260+
* the Feature is immutable.
261+
*
259262
* @param feature the GeoJSON {@link Feature} to set
260263
*/
261264
public void setGeoJson(Feature feature) {
@@ -270,6 +273,9 @@ public void setGeoJson(Feature feature) {
270273
* Updates the GeoJson with a single geometry. The update is performed asynchronously,
271274
* so the data won't be immediately visible or available to query when this method returns.
272275
*
276+
* Note: This method is not thread-safe. The Geometry is parsed on a worker thread, please make sure
277+
* the Geometry is immutable.
278+
*
273279
* @param geometry the GeoJSON {@link Geometry} to set
274280
*/
275281
public void setGeoJson(Geometry geometry) {
@@ -284,6 +290,9 @@ public void setGeoJson(Geometry geometry) {
284290
* Updates the GeoJson. The update is performed asynchronously,
285291
* so the data won't be immediately visible or available to query when this method returns.
286292
*
293+
* Note: This method is not thread-safe. The FeatureCollection is parsed on a worker thread, please make sure
294+
* the FeatureCollection is immutable.
295+
*
287296
* @param featureCollection the GeoJSON FeatureCollection
288297
*/
289298
public void setGeoJson(@Nullable FeatureCollection featureCollection) {

MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/location/LocationLayerControllerTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,14 +536,15 @@ public void onNewLatLngValue_locationFeatureIsUpdated() {
536536
LayerBitmapProvider bitmapProvider = mock(LayerBitmapProvider.class);
537537
LocationComponentOptions options = mock(LocationComponentOptions.class);
538538
Feature locationFeature = mock(Feature.class);
539+
when(locationFeature.toJson()).thenReturn("expected_geojson_string");
539540
LocationLayerController layer = new LocationLayerController(
540541
mapboxMap, mapboxMap.getStyle(), sourceProvider, buildFeatureProvider(locationFeature, options),
541542
bitmapProvider, options, internalRenderModeChangedListener, internalIndicatorPositionChangedListener, false);
542543

543544
getAnimationListener(ANIMATOR_LAYER_LATLNG, layer.getAnimationListeners()).onNewAnimationValue(new LatLng());
544545

545546
// wanted twice (once for initialization)
546-
verify(locationSource, times(2)).setGeoJson(locationFeature);
547+
verify(locationSource, times(2)).setGeoJson("expected_geojson_string");
547548
verify(internalIndicatorPositionChangedListener).onIndicatorPositionChanged(any(Point.class));
548549
}
549550

0 commit comments

Comments
 (0)