sharedThreadPool reference in platform specific implementations

[LukasPaczos]

DraggableMarkerActivity crashes on master during startup with this exception:

java.lang.Error: mutex lock failed: Device or resource busy
    at com.mapbox.mapboxsdk.style.sources.GeoJsonSource.nativeSetFeatureCollection(Native Method)
    at com.mapbox.mapboxsdk.style.sources.GeoJsonSource.setGeoJson(GeoJsonSource.java:220)
    at com.mapbox.mapboxsdk.style.sources.GeoJsonSource.<init>(GeoJsonSource.java:124)
    at com.mapbox.mapboxsdk.testapp.activity.style.DraggableMarkerActivity.<init>(DraggableMarkerActivity.kt:49)
    at java.lang.Class.newInstance(Native Method)
    at android.app.AppComponentFactory.instantiateActivity(AppComponentFactory.java:69)
    at android.app.Instrumentation.newActivity(Instrumentation.java:1215)
    at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2831)
    at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3048)

/cc @tobrun

[LukasPaczos]

Initial debugging shows that creating sources before the map is initialized is broken and results in the above crash or freeze. This regression was introduced with the async GeoJsonSource features conversion in #12580.

[LukasPaczos]

I'm seeing mutex lock failed: Device or resource busy when trying to lock mutex after invoking the actor but only when the map is not created. There seems to be no issues scheduling the actor after the map is initialized.

[jfirebaugh]

The GeoJSONSource constructors call std::make_unique<Actor<FeatureConverter>>(*sharedThreadPool()), dereferencing the std::shared_ptr<ThreadPool> that sharedThreadPool() returns. But as soon as this line is complete, the shared_ptr is destroyed, and it's the last outstanding reference, so the ThreadPool is destroyed and the actor is left with a dangling reference.

cc @kkaefer

[jfirebaugh]

CustomGeometrySource has the same issue.

[kkaefer]

They shouldn't reference sharedThreadPool to begin with. It's a platform-specific implementation and will e.g. not be available in the node bindings.

[LukasPaczos]

What are the next steps here? Should we keep the reference to the pointer instead of dereferencing it? Is a separate thread pool implementation required as of now?

Ideally, the fix would need to land before the 6.5.0 release.

[jfirebaugh]

They shouldn't reference sharedThreadPool to begin with. It's a platform-specific implementation and will e.g. not be available in the node bindings.

This is the Android-specific GeoJSONSource class, so we're okay on that front at least.

[kkaefer]

🤦‍♂️