Mapbox-gl-native: Android: JNI error: can't call java.lang.String com.mapbox.geojson.Feature.id() on null object

[jimmyjzemoso]

I am dragging a feature by updating it. When I rapidly move the feature this error happens and the app crashes:

JNI DETECTED ERROR IN APPLICATION: can't call java.lang.String com.mapbox.geojson.Feature.id() on null object
This crash is not happening in v6.3.0. This started happening when I switched from 6.3.0 to 7.3.2

This is the whole log:

2019-05-02 15:14:14.003 12250-12337/com.a.app.dev E/Mbgl: {GLThread 5845}[OpenGL]: stencil mask overflow
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] JNI DETECTED ERROR IN APPLICATION: can't call java.lang.String com.mapbox.geojson.Feature.id() on null object
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] in call to CallObjectMethodV
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] "Thread-2236" prio=10 tid=39 Runnable
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] | group="main" sCount=0 dsCount=0 flags=0 obj=0x19f80000 self=0x787bcda800
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] | sysTid=12336 nice=-10 cgrp=default sched=0/0 handle=0x7876cf94f0
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] | state=R schedstat=( 1263831607 63282496 1239 ) utm=121 stm=5 core=5 HZ=100
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] | stack=0x7876bfe000-0x7876c00000 stackSize=1009KB
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] | held mutexes= "mutator lock"(shared held)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #00 pc 00000000003c19b0 /system/lib64/libart.so (art::DumpNativeStack(std::__1::basic_ostream>&, int, BacktraceMap, char const, art::ArtMethod, void, bool)+220)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #01 pc 000000000048f10c /system/lib64/libart.so (art::Thread::DumpStack(std::__1::basic_ostream>&, bool, BacktraceMap, bool) const+352)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #02 pc 00000000002e4bb8 /system/lib64/libart.so (art::JavaVMExt::JniAbort(char const, char const)+968)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #03 pc 00000000002e4fc8 /system/lib64/libart.so (art::JavaVMExt::JniAbortV(char const, char const, std::__va_list)+116)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #04 pc 00000000000fcffc /system/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::AbortF(char const, ...)+148)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #05 pc 0000000000100ec0 /system/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::CheckMethodAndSig(art::ScopedObjectAccess&, _jobject, _jclass, _jmethodID, art::Primitive::Type, art::InvokeType)+1592)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #06 pc 00000000000ff638 /system/lib64/libart.so (art::(anonymous namespace)::CheckJNI::CallMethodV(char const, _JNIEnv, _jobject, _jclass, _jmethodID, std::__va_list, art::Primitive::Type, art::InvokeType)+760)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #07 pc 00000000000ec354 /system/lib64/libart.so (art::(anonymous namespace)::CheckJNI::CallObjectMethodV(_JNIEnv, _jobject, _jmethodID, std::__va_list)+92)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #08 pc 0000000000037364 /data/app/com.a.app.dev-x4lq_7pYVrc3WbAVJiJC2w==/lib/arm64/libmapbox-gl.so (???)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #09 pc 00000000001a7584 /data/app/com.a.app.dev-x4lq_7pYVrc3WbAVJiJC2w==/lib/arm64/libmapbox-gl.so (???)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #10 pc 00000000001a84b8 /data/app/com.a.app.dev-x4lq_7pYVrc3WbAVJiJC2w==/lib/arm64/libmapbox-gl.so (???)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #11 pc 00000000001ca848 /data/app/com.a.app.dev-x4lq_7pYVrc3WbAVJiJC2w==/lib/arm64/libmapbox-gl.so (???)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #12 pc 00000000001ca16c /data/app/com.a.app.dev-x4lq_7pYVrc3WbAVJiJC2w==/lib/arm64/libmapbox-gl.so (???)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #13 pc 000000000008e4d4 /data/app/com.a.app.dev-x4lq_7pYVrc3WbAVJiJC2w==/lib/arm64/libmapbox-gl.so (???)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #14 pc 00000000001d1c68 /data/app/com.a.app.dev-x4lq_7pYVrc3WbAVJiJC2w==/lib/arm64/libmapbox-gl.so (???)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #15 pc 000000000008ffc8 /system/lib64/libc.so (__pthread_start(void)+36)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] native: #16 pc 0000000000023968 /system/lib64/libc.so (__start_thread+68)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542] (no managed stack frames)
2019-05-02 15:14:17.472 12250-12336/com.a.app.dev A/stresponder.de: java_vm_ext.cc:542]
2019-05-02 15:14:17.610 12250-12336/com.a.app.dev A/stresponder.de: runtime.cc:558] Runtime aborting...
2019-05-02 15:14:17.610 12250-12336/com.a.app.dev A/stresponder.de: runtime.cc:558] Dumping all threads without appropriate locks held: thread list lock mutator lock
.........
2019-05-02 15:14:17.622 12250-12336/com.a.app.dev A/libc: Fatal signal 6 (SIGABRT), code -6 (SI_TKILL) in tid 12336 (Thread-2236), pid 12250 (stresponder.dev)

I also noticed a sudden drop in memory usage from 160 to 75mb in the android profiler just before the crash.

This is what I am doing while dragging:

   `@Override
    public boolean onMove(MoveGestureDetector detector, float distanceX, float distanceY) {
        List<Feature> features = symbolsCollection.features();

            if (features != null) {
                Feature oldFeature = getFeatureWithId(features, draggedSymbolId);
                if (oldFeature != null) {
                    JsonObject oldProperties = oldFeature.properties();
                    Feature newFeature = null;
                    Geometry geometry = oldFeature.geometry();

                    if(geometry instanceof Point) {
                        if (oldProperties != null && oldProperties.has(STICK_EDGE)) {
                            newFeature = Feature.fromGeometry(Point.fromLngLat(latLng.getLongitude(),
                                    latLng.getLatitude()), oldProperties, draggedSymbolId);
                            Feature lineString = getFeatureWithId(features, STICK_EDGE_ID);
                            if(lineString!=null && lineString.geometry() instanceof LineString) {
                                LineString coordinates = (LineString) lineString.geometry();
                                Point oldVertex = (Point) oldFeature.geometry();
                                Point newVertex = Point.fromLngLat(latLng.getLongitude(), latLng.getLatitude());
                                assert coordinates != null;
                                List<Point> coords = coordinates.coordinates();
                                List<Point> pts = new ArrayList<>();
                                for(int i=0;i<coords.size();i++)
                                    if(coords.get(i).equals(oldVertex))
                                        pts.add(newVertex);
                                    else
                                        pts.add(coords.get(i));
                                Feature newLineString = Feature.fromGeometry(LineString.fromLngLats(pts), oldProperties, AppConstants.MapBox.STICK_EDGE);
                                features.set(features.indexOf(lineString), newLineString);
                                features.set(features.indexOf(oldFeature), newFeature);
                            }
                        }
                        if (oldProperties != null && !oldProperties.has(FEATURE_FILL) && !oldProperties.has(STICK_EDGE)) {
                            newFeature = Feature.fromGeometry(Point.fromLngLat(latLng.getLongitude(),
                                    latLng.getLatitude()), oldProperties, draggedSymbolId);
                            features.set(features.indexOf(oldFeature), newFeature);
                        }
    }
   private static @Nullable Feature getFeatureWithId(List<Feature> features, String id) {
        if (features != null) {
            for (Feature feature : features) {
                String featureId = feature.id();
                if (featureId != null && featureId.equals(id)) {
                    return feature;
                }
            }
        }
        return null;
    }`

Expected behavior

The app should not crash

Actual behavior

The app is crashing with JNI error.

Configuration

Android versions: Android Pie
Device models: One plus 5, Pixel 2
Mapbox SDK versions: 7.3.0, 7.3.2

[LukasPaczos]

Thanks for reaching out @jimmyjzemoso. Unfortunately, from the provided code I'm unable to judge whether there's a chance of the feature actually being null. There's no code that does the actual source updates in there. I also tried reproducing using the DraggableMarkerActivity while constantly clearing the memory and running the GC. No luck.

Would you be able to provide a small, self-contained reproducible example?

[jimmyjzemoso]

Hi @LukasPaczos thanks for getting back. I will try to provide such an example.

[jimmyjzemoso]

Hi @LukasPaczos I have made a sample app and added you and @tobrun as a collaborator.
https://github.com/jimmyjzemoso/MapboxCrash

Please tap on the map to add a polygon and drag it to invoke the crash.
The crash is intermittent so I am adding a screenrecording of how to invoke the crash: https://drive.google.com/file/d/1KacBcAS4n7iEXc9yF_jGavc-G7Z8Io-O/view?usp=sharing

[jimmyjzemoso]

Hi @LukasPaczos I wanted to add current location marker with locationlayer plugin on 6.3.2. The same crash was happening in to 6.3.2 if we add locationlayerplugin and drag the map.

[LukasPaczos]

Thanks for the example project @jimmyjzemoso! I was able to reproduce the issue and the root cause is our internal feature conversion. The conversion from Java to C++ features is asynchronous, and because you are often modifying and clearing the original Java collection, it can be cleared and nullify all of the records while the conversion is happening on a worker thread.

To fix the issue without modifying your current logic, whenever you update a GeoJsonSource, copy the feature collection first so that a new object is passed to the core and you can freely modify your original collection. For example instead of calling:

symbolsSource.setGeoJson(symbolsCollection);

call

symbolsSource.setGeoJson(FeatureCollection.fromFeatures(new ArrayList<>(symbolsCollection.features())));

I verified that this resolves your particular issue and can serve as a workaround, but since the feature conversion is not documented and would've been hard to synchronize, I'm also going to follow up with a PR that copies the collection when it's passed to the core by default.

I wanted to add current location marker with locationlayer plugin on 6.3.2. The same crash was happening in to 6.3.2 if we add locationlayerplugin and drag the map.

Unfortunately, the LocationLayerPlugin is not supported and I'd recommend migrating to a verions > v7.x and use the LocationComponent.

[jimmyjzemoso]

@LukasPaczos Thank you for the update. I will test it and let you know.
Btw if this was the issue, why was it not happening in the older version of the SDK (it's not happening in v6.3.2)

[LukasPaczos]

That's because we've only introduced the async features conversion in the 7x. I'm still looking into details to build a regression test.

[jimmyjzemoso]

@LukasPaczos The workaround is working. Thanks.

Will this change be added in the next release??

[tobrun]

yes, this fix can be tested with the 4.1.0-alpha.1 that will be released later today.
Final version lands in 2 weeks.