Crash when changing styleURL if there is an empty datasource?

[nitrag]

I get a hard crash when changing map base layers -- I THINK -- when you have an empty data source? Seems to crash when re-adding a GeoJSON data source.

Actual:
If you don't comment out the line below, the app will crash when you click the button to change the map base layer.

import UIKit
import Mapbox

class ViewController: UIViewController, MGLMapViewDelegate {

    var mapView: MGLMapView!
    @IBOutlet weak var mapContainer: UIView!
    var geojsonDataSource: MGLGeoJSONSource = MGLGeoJSONSource(identifier: "layer-source", features: [MGLFeature](), options: nil)
    var newStyle: MGLLineStyleLayer? = nil
    
    override func viewDidLoad() {
        super.viewDidLoad()
        mapView = MGLMapView(frame: view.bounds)
        mapView.autoresizingMask = [.flexibleWidth, .flexibleHeight]
        mapView.delegate = self
        mapContainer.addSubview(mapView)
        mapView.showsUserLocation = true
    }
    func mapViewDidFinishLoadingMap(_ mapView: MGLMapView) {
        
        //BUG BUG BUG...can't switch style layers if there is an empty source?
-->        self.mapView.style().add(self.geojsonDataSource)
        //END BUG, comment out to not have issues when changeStyle
        
        self.newStyle = MGLLineStyleLayer(identifier: "layer-style", source: self.geojsonDataSource)
        self.newStyle?.lineColor = MGLStyleConstantValue(rawValue: UIColor.blue)
        self.newStyle?.lineWidth = MGLStyleConstantValue(rawValue: 4)
        self.newStyle?.lineOpacity = MGLStyleConstantValue(rawValue: 0.8)
        self.mapView.style().add(self.newStyle!)
    }
    @IBAction func changeStyle(_ sender: Any) {
        mapView.styleURL = URL(string: "mapbox://styles/mapbox/satellite-streets-v9")
    }
}

Mapbox`::-[MGLGeoJSONSource addToMapView:](MGLMapView *):
    0x10106d7a6 <+0>:   pushq  %rbp
    0x10106d7a7 <+1>:   movq   %rsp, %rbp
    0x10106d7aa <+4>:   pushq  %rbx
    0x10106d7ab <+5>:   pushq  %rax
    0x10106d7ac <+6>:   movq   %rdi, %rbx
    0x10106d7af <+9>:   movq   0x6dcf52(%rip), %rsi      ; "mbglMap"
    0x10106d7b6 <+16>:  movq   %rdx, %rdi
    0x10106d7b9 <+19>:  callq  *0x6be459(%rip)           ; (void *)0x0000000103ec5ac0: objc_msgSend
    0x10106d7bf <+25>:  movq   0x6df4a2(%rip), %rcx
    0x10106d7c6 <+32>:  movq   (%rbx,%rcx), %rdx
    0x10106d7ca <+36>:  movq   $0x0, (%rbx,%rcx)
    0x10106d7d2 <+44>:  movq   %rdx, -0x10(%rbp)
    0x10106d7d6 <+48>:  leaq   -0x10(%rbp), %rsi
    0x10106d7da <+52>:  movq   %rax, %rdi
    0x10106d7dd <+55>:  callq  0x1012cc720               ; mbgl::Map::addSource(std::__1::unique_ptr<mbgl::style::Source, std::__1::default_delete<mbgl::style::Source> >)
    0x10106d7e2 <+60>:  movq   -0x10(%rbp), %rdi
    0x10106d7e6 <+64>:  movq   $0x0, -0x10(%rbp)
    0x10106d7ee <+72>:  testq  %rdi, %rdi
    0x10106d7f1 <+75>:  je     0x10106d7f9               ; <+83> at MGLGeoJSONSource.mm:66
    0x10106d7f3 <+77>:  movq   (%rdi), %rax
    0x10106d7f6 <+80>:  callq  *0x8(%rax)
    0x10106d7f9 <+83>:  addq   $0x8, %rsp
    0x10106d7fd <+87>:  popq   %rbx
    0x10106d7fe <+88>:  popq   %rbp
    0x10106d7ff <+89>:  retq   
    0x10106d800 <+90>:  movq   %rax, %rbx
    0x10106d803 <+93>:  jmp    0x10106d81f               ; <+121> at memory:2516
    0x10106d805 <+95>:  movq   %rax, %rbx
    0x10106d808 <+98>:  movq   -0x10(%rbp), %rdi
    0x10106d80c <+102>: movq   $0x0, -0x10(%rbp)
    0x10106d814 <+110>: testq  %rdi, %rdi
    0x10106d817 <+113>: je     0x10106d81f               ; <+121> at memory:2516
    0x10106d819 <+115>: movq   (%rdi), %rax
    0x10106d81c <+118>: callq  *0x8(%rax)
    0x10106d81f <+121>: movq   %rbx, %rdi
    0x10106d822 <+124>: callq  0x10169f2c0               ; symbol stub for: _Unwind_Resume

Why do I have an empty data geojson layer? Well, I create an empty repository and add it to the map, then as a user scrolls to a particular area data is populated from the internet. It may be empty for the entirety of the application too.

Here is the sample app, without Mapbox Pod:
MapboxBUG-ChangeStyle.zip

[boundsj]

Thanks for sending the sample app. However, I'm unable to reproduce the crash with beta.3. You should indeed be able to create a MGLGeoJSONSource instance with an empty features array and then set that instance's features property to some other value later on. There is an example of this sort of thing in the Objective-C iosapp in the repo itself.

[nitrag]

I still have this issue with Beta 4. You can't reproduce with the sample app?

[boundsj]

@nitrag You can't reproduce with the sample app?

I do see the issue now with your code. You are adding the same source (and layer) instances each time mapViewDidFinishLoadingMap is called after a style URL change. You cannot add the same source an layer instances to the style multiple times. You can remove sources and layers first before adding them back #7048 (comment) but it'd be better to introduce a guard in your application code so that once a layer or source is added, you don't attempt to add the same one again.

Relatedly, it is also not a good idea to use the same identifier for source and layer objects across distinct instances no matter the sub-type. In fact, in the next iOS beta for 3.4.0, reusing identifiers will trigger an exception for sources (#7203) and layers (#7257).

This bug report is a good reminder that we should raise an NSException if source and layer instances are added more than once and also improve the related API docs in MGLSource. I opened #7271 to track this.

Note: It is fine to add a source with no features:

    func mapViewDidFinishLoadingMap(_ mapView: MGLMapView) {
        if (alreadyAdded) { return }
        let source = MGLGeoJSONSource(identifier: "source", features: [], options: nil)
        mapView.style().add(source)
        alreadyAdded = true
    }

And you can add features later on:

    func mapViewDidFinishLoadingMap(_ mapView: MGLMapView) {
       if (alreadyAdded) { return }

        let source = MGLGeoJSONSource(identifier: "source", features: [], options: nil)
        mapView.style().add(source)

        let layer = MGLCircleStyleLayer(identifier: "layer", source: source)
        layer.circleColor = MGLStyleValue(rawValue: .red)
        layer.circleRadius = MGLStyleValue(rawValue: 100)
        mapView.style().add(layer)

        alreadyAdded = true

        DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 2) {
            let feature = MGLPointFeature()
            feature.coordinate = CLLocationCoordinate2D(latitude: 40.73581, longitude: -73.99155)
            source.features = [feature]
        }
    }

I'm going to close this and we can track the related work in:

#7271
#7203
#7257

[nitrag]

@boundsj, Something I just realized and can't find a clear answer to in the documentation.

You cannot add the same source instances to the style multiple times

How are we supposed to handle changing the base StyleURL and existing MGLSources/MGLStyleLayers?

I've implemented alreadyAdded technique to load the layers/sources once, EVER. When I change the base StyleURL all my vector layers on top of the map disappear.

To get them to re-appear again I'm setting alreadyAdded=false right before a style change. This fixes the problem but I'm worried I'm doing it wrong by re-init'ing the MGLSource's.

So, currently:

var alreadyAdded = false
func changeMapStyle() {
	self.alreadyAdded = false //add this?
	self.mapView.styleURL = newStyle
}
func mapViewDidFinishLoadingMap(_ mapView: MGLMapView) {

    //not sure if there is ever a case when this would be true
    if(self.alreadyAdded == false){ 
	    
        //does this even need to be a VC-level variable? I don't seem to ever need to reference it...
        self.trailVector = MGLVectorSource(identifier: "trail-data", configurationURL: URL(string: styleUrl)!) 

        style.addSource(self.trailVector)
    }

    //add layers every time
    let styleLayer = MGLLineStyleLayer(identifier: layer.title, source: self.trailVector)
    ...
    style.addLayer(styleLayer)
    ....    
}

is at least working.

Should I be nulling/removing the MGLSource's before changing the StyleUrl? Does mapbox automatically remove/dereference them? It just seems odd that MGLSource's can't be re-used and stick to the MGLMapView itself.

[nitrag]

Particular use case: What if I have a MGLShapeSource that I'm continually modifying an in-progress recorded GPS track? I would need that to live through style changes, if say the user wanted to look at a satellite view real quick and flip back. It would make most sense to not touch the MGLShapeSource and just re-apply the MGLLineStyleLayer to the new style in mapViewDidFinishLoadingMap()

[boundsj]

Hi @nitrag. The feature you are asking about persist layers and sources across styles is tracked in #6180.

Your current workaround seems reasonable to me. You can continue to recreate the source (and you don't need to store it in a class property) because you can't reuse a source once it is added to a style (even a different style) if you don't remove it first because of the transfer of ownership of the source to the rendering layer of the SDK. If there is performance gain to be had from caching the source, you could continue to store it as a property and try removing it from the current style before you change to the new style and add the source again. Removing a source (or layer) transfers the ownership of the instance back from the rendering layer of the SDK to the instance that your application (in this case) keeps a reference to.

[nitrag]

PSA: Don't use class variables for sources and layers. It'll open a can of 💩 . Instead, just re-init in mapViewDidFinishLoadingMap as you did initially. Awaiting PR #6180