From f3c29ccf0b21353e8e64de5c50a5bf682a9526fb Mon Sep 17 00:00:00 2001 From: liyanjin <36877356+grab-liyanjin@users.noreply.github.com> Date: Wed, 27 May 2026 10:03:21 +0800 Subject: [PATCH] fix(android): guard stale layer remove path (#4300) Co-authored-by: Cursor --- .../src/cpp/native_map_view.cpp | 15 ++++-- .../maplibre/android/maps/NativeMapView.java | 12 ++++- .../testapp/style/RuntimeStyleTests.java | 49 +++++++++++++++++++ 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/platform/android/MapLibreAndroid/src/cpp/native_map_view.cpp b/platform/android/MapLibreAndroid/src/cpp/native_map_view.cpp index c351840a8..935b170ce 100644 --- a/platform/android/MapLibreAndroid/src/cpp/native_map_view.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/native_map_view.cpp @@ -1154,11 +1154,20 @@ jni::jboolean NativeMapView::removeLayerAt(JNIEnv& env, jni::jint index) { /** * Remove with wrapper object id. Ownership is transferred back to the wrapper */ -jni::jboolean NativeMapView::removeLayer(JNIEnv&, jlong layerPtr) { - assert(layerPtr != 0); +jni::jboolean NativeMapView::removeLayer(JNIEnv& env, jlong layerPtr) { + if (layerPtr == 0) { + Log::Warning(Event::JNI, "Cannot remove layer: native layer pointer is null"); + return jni::jni_false; + } mbgl::android::Layer* layer = reinterpret_cast(layerPtr); - std::unique_ptr coreLayer = map->getStyle().removeLayer(layer->get().getID()); + const auto layerId = jni::Make(env, layer->getId(env)); + if (layerId.empty()) { + Log::Warning(Event::JNI, "Cannot remove layer: layer reference is detached or invalid"); + return jni::jni_false; + } + + std::unique_ptr coreLayer = map->getStyle().removeLayer(layerId); if (coreLayer) { layer->setLayer(std::move(coreLayer)); return jni::jni_true; diff --git a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/NativeMapView.java b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/NativeMapView.java index 721de41d3..572564713 100755 --- a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/NativeMapView.java +++ b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/NativeMapView.java @@ -1000,7 +1000,17 @@ final class NativeMapView implements NativeMap { if (checkState("removeLayer")) { return false; } - return nativeRemoveLayer(layer.getNativePtr()); + if (layer.isDetached()) { + Logger.w(TAG, "Ignoring removeLayer() call on detached layer reference."); + return false; + } + + final long nativePtr = layer.getNativePtr(); + if (nativePtr == 0L) { + Logger.w(TAG, "Ignoring removeLayer() call on released layer pointer."); + return false; + } + return nativeRemoveLayer(nativePtr); } @Override diff --git a/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/RuntimeStyleTests.java b/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/RuntimeStyleTests.java index 8a97568d5..216c5cac8 100644 --- a/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/RuntimeStyleTests.java +++ b/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/RuntimeStyleTests.java @@ -9,6 +9,7 @@ import androidx.test.espresso.ViewAction; import androidx.test.internal.runner.junit4.AndroidJUnit4ClassRunner; import org.maplibre.android.style.layers.CannotAddLayerException; +import org.maplibre.android.style.layers.BackgroundLayer; import org.maplibre.android.style.layers.CircleLayer; import org.maplibre.android.style.layers.FillLayer; import org.maplibre.android.style.layers.Layer; @@ -22,6 +23,7 @@ import org.maplibre.android.style.sources.Source; import org.maplibre.android.style.sources.VectorSource; import org.maplibre.android.testapp.R; import org.maplibre.android.testapp.activity.EspressoTest; +import org.maplibre.android.maps.Style; import org.hamcrest.Matcher; import org.junit.Assert; @@ -300,6 +302,53 @@ public class RuntimeStyleTests extends EspressoTest { }); } + @Test + public void testRemoveStaleLayerReferenceDoesNotCrash() { + validateTestSetup(); + onView(withId(R.id.mapView)).perform(new BaseViewAction() { + @Override + public void perform(UiController uiController, View view) { + final Layer[] staleLayerHolder = new Layer[1]; + final Throwable[] errorHolder = new Throwable[1]; + final Boolean[] removeResultHolder = new Boolean[1]; + final boolean[] finished = new boolean[] { false }; + + Style.Builder styleABuilder = new Style.Builder() + .withLayer(new BackgroundLayer("stale-layer-a") + .withProperties(PropertyFactory.backgroundColor(Color.BLUE))); + Style.Builder styleBBuilder = new Style.Builder() + .withLayer(new BackgroundLayer("style-b-background") + .withProperties(PropertyFactory.backgroundColor(Color.DKGRAY))); + + maplibreMap.setStyle(styleABuilder, styleA -> { + staleLayerHolder[0] = styleA.getLayer("stale-layer-a"); + + maplibreMap.setStyle(styleBBuilder, styleB -> { + try { + removeResultHolder[0] = styleB.removeLayer(staleLayerHolder[0]); + } catch (Throwable error) { + errorHolder[0] = error; + } finally { + finished[0] = true; + } + }); + }); + + long timeoutMs = 5000; + long startMs = System.currentTimeMillis(); + while (!finished[0] && System.currentTimeMillis() - startMs < timeoutMs) { + uiController.loopMainThreadForAtLeast(50); + } + + assertTrue("Style switch did not finish in time", finished[0]); + assertNotNull("Stale layer should have been captured", staleLayerHolder[0]); + assertNull("Removing stale layer reference should not throw", errorHolder[0]); + assertFalse("Removing stale layer reference should fail gracefully", + Boolean.TRUE.equals(removeResultHolder[0])); + } + }); + } + /** * https://github.com/mapbox/mapbox-gl-native/issues/7973 */